-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding created_at
, last_modified_at
, and read-only fields
#4597
Conversation
WalkthroughSignificant updates have been made to the FiftyOne framework, focusing on frame metadata handling and improved dataset management. Enhancements include new parameters for better timestamp tracking, refined validation processes, a migration script for version 0.26.0, and version updates across related modules, all aimed at improving dataset integrity and usability. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
if sets: | ||
updates["$set"] = sets | ||
try: | ||
# OPTIMIZATION: we monkey patch `to_mongo()` and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for this, I couldn't help myself...
This significantly improves the efficiency of sample.save()
if a sample contains many large fields but you've only made a simple edit to one field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (4)
fiftyone/core/frame.py (1)
798-824
: Improve exception handling.Consider using
raise ... from err
orraise ... from None
to distinguish exceptions from errors in exception handling.- except Exception as e: + except Exception as e: + raise ValueError( + "Invalid value for field '%s' of frame %d. " + "Reason: %s" + % (field_name, frame.frame_number, str(e)) + ) from eTools
Ruff
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
fiftyone/core/fields.py (2)
Line range hint
59-111
: Simplify nestedif
statements.The nested
if
statements can be combined for better readability.- if read_only is not None: - if not isinstance(read_only, bool): - raise ValueError("read_only must be a boolean") + if read_only is not None and not isinstance(read_only, bool): + raise ValueError("read_only must be a boolean")
Line range hint
152-214
: Simplify nestedif
statements.The nested
if
statements can be combined for better readability.- if read_only is not None: - if read_only != field.read_only: - raise ValueError( - "%s has read_only %s, not %s" - % (_make_prefix(path), field.read_only, read_only) - ) + if read_only is not None and read_only != field.read_only: + raise ValueError( + "%s has read_only %s, not %s" + % (_make_prefix(path), field.read_only, read_only) + )tests/unittests/import_export_tests.py (1)
3622-3622
: Ensure consistency in test assertionsThe new assertions for
created_at
andlast_modified_at
are present intests/unittests/import_export_tests.py
but are missing in the following files:
tests/unittests/group_tests.py
tests/intensive/import_export_tests.py
Please add the new assertions to these files to ensure consistency in testing the
test_legacy_fiftyone_dataset
function.
Analysis chain
LGTM! But verify the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
test_legacy_fiftyone_dataset
are updated to include the new assertions.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `test_legacy_fiftyone_dataset` include the new assertions. # Test: Search for the function usage. Expect: Only occurances of the new assertions. rg --type python -A 5 $'test_legacy_fiftyone_dataset'Length of output: 83
Script:
#!/bin/bash # Description: Verify all function calls to `test_legacy_fiftyone_dataset` include the new assertions. # Test: Search for the function usage. Expect: Only occurrences of the new assertions. rg --type py -A 5 $'test_legacy_fiftyone_dataset'Length of output: 2156
Script:
#!/bin/bash # Description: Verify the presence of the new assertions in the identified files. # Check for `created_at` and `last_modified_at` in `tests/unittests/group_tests.py` rg --type py -A 5 'created_at' tests/unittests/group_tests.py # Check for `created_at` and `last_modified_at` in `tests/unittests/import_export_tests.py` rg --type py -A 5 'created_at' tests/unittests/import_export_tests.py # Check for `created_at` and `last_modified_at` in `tests/intensive/import_export_tests.py` rg --type py -A 5 'created_at' tests/intensive/import_export_tests.pyLength of output: 3171
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (37)
- docs/source/cheat_sheets/views_cheat_sheet.rst (5 hunks)
- docs/source/integrations/coco.rst (1 hunks)
- docs/source/user_guide/app.rst (1 hunks)
- docs/source/user_guide/basics.rst (6 hunks)
- docs/source/user_guide/evaluation.rst (1 hunks)
- docs/source/user_guide/groups.rst (15 hunks)
- docs/source/user_guide/using_datasets.rst (39 hunks)
- docs/source/user_guide/using_views.rst (9 hunks)
- fiftyone/core/clips.py (4 hunks)
- fiftyone/core/collections.py (33 hunks)
- fiftyone/core/dataset.py (67 hunks)
- fiftyone/core/document.py (3 hunks)
- fiftyone/core/fields.py (48 hunks)
- fiftyone/core/frame.py (10 hunks)
- fiftyone/core/odm/dataset.py (5 hunks)
- fiftyone/core/odm/document.py (11 hunks)
- fiftyone/core/odm/embedded_document.py (2 hunks)
- fiftyone/core/odm/frame.py (2 hunks)
- fiftyone/core/odm/mixins.py (31 hunks)
- fiftyone/core/odm/sample.py (2 hunks)
- fiftyone/core/odm/utils.py (5 hunks)
- fiftyone/core/patches.py (2 hunks)
- fiftyone/core/stages.py (9 hunks)
- fiftyone/core/video.py (3 hunks)
- fiftyone/core/view.py (10 hunks)
- fiftyone/migrations/revisions/v0_25_0.py (1 hunks)
- fiftyone/utils/data/importers.py (5 hunks)
- package/desktop/setup.py (1 hunks)
- setup.py (2 hunks)
- tests/unittests/dataset_tests.py (48 hunks)
- tests/unittests/group_tests.py (8 hunks)
- tests/unittests/import_export_tests.py (3 hunks)
- tests/unittests/index_tests.py (4 hunks)
- tests/unittests/patches_tests.py (6 hunks)
- tests/unittests/run_tests.py (1 hunks)
- tests/unittests/video_tests.py (22 hunks)
- tests/unittests/view_tests.py (6 hunks)
Files skipped from review due to trivial changes (2)
- package/desktop/setup.py
- setup.py
Additional context used
Ruff
fiftyone/core/odm/dataset.py
9-9:
datetime.datetime
imported but unusedRemove unused import:
datetime.datetime
(F401)
fiftyone/core/frame.py
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
fiftyone/core/fields.py
107-108: Use a single
if
statement instead of nestedif
statementsCombine
if
statements usingand
(SIM102)
208-209: Use a single
if
statement instead of nestedif
statementsCombine
if
statements usingand
(SIM102)
fiftyone/core/odm/mixins.py
10-10:
itertools
imported but unusedRemove unused import:
itertools
(F401)
122-124: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
296-296: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
tests/unittests/dataset_tests.py
1515-1515: Do not assign a
lambda
expression, use adef
Rewrite
key_fcn
as adef
(E731)
1584-1584: Found useless expression. Either assign it to a variable or remove it.
(B018)
1586-1586: Found useless expression. Either assign it to a variable or remove it.
(B018)
2756-2756: Found useless expression. Either assign it to a variable or remove it.
(B018)
2767-2767: Found useless expression. Either assign it to a variable or remove it.
(B018)
3047-3047: Found useless expression. Either assign it to a variable or remove it.
(B018)
3058-3058: Found useless expression. Either assign it to a variable or remove it.
(B018)
fiftyone/core/dataset.py
3410-3410: Ambiguous variable name:
l
(E741)
3412-3412: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
3609-3612: Use ternary operator
doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls
instead ofif
-else
-blockReplace
if
-else
-block withdoc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls
(SIM108)
fiftyone/core/collections.py
652-652: Do not use bare
except
(E722)
Additional comments not posted (227)
fiftyone/core/odm/frame.py (3)
26-27
: LGTM!The addition of
created_at
andlast_modified_at
fields as read-onlyDateTimeField
enhances the functionality by tracking creation and modification times.
44-45
: LGTM!Initializing
created_at
andlast_modified_at
fields withNone
ensures they are included in the instance's keyword arguments.
53-54
: LGTM!Including
created_at
andlast_modified_at
fields in the_data
dictionary ensures they are part of the document's data structure.fiftyone/core/odm/embedded_document.py (1)
Line range hint
51-66
:
LGTM!The addition of the
allow_missing
parameter to the_get_field
method provides more flexibility in field retrieval by allowing optional handling of missing fields.fiftyone/migrations/revisions/v0_25_0.py (3)
10-26
: LGTM!The
up
function correctly adds thelast_modified_at
property to the dataset and updates sample and frame fields.
29-30
: LGTM!The
down
function serves as a placeholder for potential rollback logic.
33-76
: LGTM!The
_up_fields
function correctly updates fields to include theread_only
property and addscreated_at
andlast_modified_at
fields if they are missing.tests/unittests/index_tests.py (4)
31-34
: LGTM! Addedcreated_at
index intest_image
.The addition of the
created_at
index is correct and consistent with the existing index definitions.
35-38
: LGTM! Addedlast_modified_at
index intest_image
.The addition of the
last_modified_at
index is correct and consistent with the existing index definitions.
59-66
: LGTM! Addedcreated_at
andlast_modified_at
indices intest_group
.The additions of the
created_at
andlast_modified_at
indices are correct and consistent with the existing index definitions.
Line range hint
95-123
:
LGTM! Addedcreated_at
andlast_modified_at
indices intest_video
.The additions of the
created_at
andlast_modified_at
indices are correct and consistent with the existing index definitions.fiftyone/core/odm/sample.py (2)
88-89
: LGTM! Addedcreated_at
andlast_modified_at
fields inDatasetSampleDocument
.The additions of the
created_at
andlast_modified_at
fields as read-onlyDateTimeField
are correct and consistent with the existing field definitions.
120-121
: LGTM! Initializedcreated_at
andlast_modified_at
fields inNoDatasetSampleDocument
.The initialization of the
created_at
andlast_modified_at
fields toNone
is correct and consistent with the existing field initializations.tests/unittests/run_tests.py (1)
173-191
: LGTM! Addedtest_run_timestamps
method.The addition of the
test_run_timestamps
method to validate timestamp behavior when cloning a dataset is correct and consistent with the existing test methods.docs/source/user_guide/basics.rst (4)
80-85
: LGTM! Added fields enhance metadata management.The addition of
created_at
andlast_modified_at
fields improves the functionality by allowing users to manage and query samples based on their temporal metadata.
125-126
: LGTM! Consistent inclusion of new fields.The addition of
created_at
andlast_modified_at
fields to the default set of fields for samples ensures consistency and enhances metadata management.
145-146
: LGTM! Updated example to reflect new fields.The addition of
created_at
andlast_modified_at
fields to the sample print output example ensures that the documentation accurately reflects the current state of the code.
202-207
: LGTM! Consistent updates across examples.The addition of
created_at
andlast_modified_at
fields to this sample print output example maintains consistency across the documentation and ensures accuracy.fiftyone/core/odm/utils.py (4)
185-185
: LGTM! Addedread_only
parameter enhances functionality.The addition of the
read_only
parameter to thecreate_field
function signature allows users to specify whether a field should be read-only, enhancing the functionality.
217-217
: LGTM! Updated docstring for clarity.The addition of the
read_only
parameter to the docstring of thecreate_field
function ensures that the documentation is clear and up-to-date.
229-233
: LGTM! Ensures proper handling ofread_only
attribute.The addition of the
read_only
parameter to thefield_kwargs
dictionary ensures that theread_only
attribute is appropriately passed along when creating field instances.
315-315
: LGTM! Ensures accessibility ofread_only
property.The addition of the
read_only
attribute to the returned dictionary in theget_field_kwargs
and_get_field_kwargs
functions ensures that theread_only
property is accessible when retrieving field metadata.Also applies to: 414-414
docs/source/integrations/coco.rst (1)
287-292
: LGTM! Added fields enhance dataset metadata.The addition of
created_at
andlast_modified_at
fields to the dataset schema improves the functionality by providing additional temporal context for each entry.docs/source/cheat_sheets/views_cheat_sheet.rst (5)
444-445
: Correctly added timestamp fields.The
created_at
andlast_modified_at
fields are correctly added to the patch fields.
490-491
: Correctly added timestamp fields.The
created_at
andlast_modified_at
fields are correctly added to the evaluation patch fields.
538-539
: Correctly added timestamp fields.The
created_at
andlast_modified_at
fields are correctly added to the clip fields.
543-544
: Correctly added timestamp fields.The
created_at
andlast_modified_at
fields are correctly added to the frame fields.
621-622
: Correctly added timestamp fields.The
created_at
andlast_modified_at
fields are correctly added to the sample fields.fiftyone/core/odm/dataset.py (4)
71-71
: Correctly addedread_only
field.The
read_only
field is correctly added to theSampleFieldDocument
class.
103-103
: Correctly integratedread_only
field into_field
method.The
read_only
field is correctly passed to thecreate_field
function in theto_field
method.
131-131
: Correctly integratedread_only
field infrom_field
method.The
read_only
field is correctly set in thefrom_field
method.
623-623
: Correctly addedlast_modified_at
field.The
last_modified_at
field is correctly added to theDatasetDocument
class.fiftyone/core/document.py (4)
244-244
: New parameterinclude_timestamps
added toiter_fields
method.The
include_timestamps
parameter is correctly added to the method signature.
250-251
: Correctly documentedinclude_timestamps
parameter.The
include_timestamps
parameter is correctly documented in the method docstring.
257-263
: Correctly integratedinclude_timestamps
logic initer_fields
method.The logic to handle
include_timestamps
parameter is correctly integrated into the method.
463-464
: Correctly excluded timestamp fields in_parse_fields
method.The
created_at
andlast_modified_at
fields are correctly excluded in the_parse_fields
method.fiftyone/core/odm/document.py (8)
328-338
: Newallow_missing
parameter in_get_field
method.The
allow_missing
parameter allows for more flexible retrieval of fields without raising an error when fields are absent. Ensure that this parameter is used appropriately in other methods to handle missing fields gracefully.
347-351
: Read-only field check inset_field
method.The read-only check ensures data integrity by preventing modifications to read-only fields. This is a crucial addition to maintain the consistency and integrity of the data.
365-369
: Read-only field check inclear_field
method.The read-only check prevents clearing of read-only fields, ensuring data integrity. This is a necessary addition to maintain the consistency and integrity of the data.
649-649
: Newenforce_read_only
parameter in_save
method.The
enforce_read_only
parameter ensures that read-only constraints are enforced during the save operation. This is a crucial addition to maintain data integrity.
756-765
: Newvirtual
parameter in_update
method.The
virtual
parameter allows for updates to be marked as virtual, which may affect how updates are tracked and applied. Ensure that this parameter is used appropriately to handle virtual updates.
775-778
: New_update_last_loaded_at
method.The
_update_last_loaded_at
method ensures that thelast_loaded_at
timestamp is consistently maintained whenever a document is updated. This is important for tracking the last load time of documents.
797-805
: New_parse_changed_fields
method.The
_parse_changed_fields
method helps in identifying the roots and paths of changed fields, which is useful for tracking updates. This method enhances the ability to manage and track field changes efficiently.
826-837
: New_validate_updates
method.The
_validate_updates
method ensures that updates are properly validated and read-only constraints are enforced, enhancing data integrity. This method is crucial for maintaining the consistency and integrity of the data.tests/unittests/patches_tests.py (3)
75-76
: Inclusion ofcreated_at
andlast_modified_at
fields intest_to_patches
.The inclusion of
created_at
andlast_modified_at
fields in various assertions ensures that the new timestamp fields are properly integrated into the data model and are subject to the same validation rules as other fields. This enhances the robustness of the tests.Also applies to: 94-102, 110-116
394-395
: Inclusion ofcreated_at
andlast_modified_at
fields intest_to_evaluation_patches
.The inclusion of
created_at
andlast_modified_at
fields in various assertions ensures that the new timestamp fields are properly integrated into the data model and are subject to the same validation rules as other fields. This enhances the robustness of the tests.Also applies to: 407-415, 423-429
754-950
: Newtest_to_patches_datetimes
method.The
test_to_patches_datetimes
method validates the behavior of thecreated_at
andlast_modified_at
fields, including checks for read-only constraints and verifying that modifications to them raise appropriate exceptions. It ensures that the timestamps behave as expected during various operations, enhancing the robustness of the tests.fiftyone/core/patches.py (2)
636-640
: Improved error handling for nested fields inmake_patches_dataset
.The improved error handling for nested fields provides clearer error messages, specifying the field name and its depth. This enhances the clarity of error reporting for users.
870-871
: Inclusion ofcreated_at
andlast_modified_at
fields in_make_patches_view
.The inclusion of the
created_at
andlast_modified_at
fields in the view configuration allows for better tracking of the lifecycle of patches. This enhances the metadata available for the patches view.fiftyone/core/video.py (3)
176-176
: LGTM!The
last_modified_at
field is correctly discarded fromsample_only_fields
to align with the new timestamp tracking functionality.
383-383
: LGTM!Including the
last_modified_at
field in the projection ensures it is part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.
641-641
: Approved with caution: Ensure careful handling.The addition of
overwrite=True
in themerge_field_schema
call allows existing fields to be overwritten, providing more flexibility in schema management. However, this requires careful handling to avoid potential data loss.docs/source/user_guide/groups.rst (6)
126-132
: LGTM!The addition of
created_at
andlast_modified_at
fields enhances the sample fields list by providing more detailed information about each sample's history.
301-307
: LGTM!The addition of
created_at
andlast_modified_at
fields enhances the sample fields list by providing more detailed information about each sample's history.
364-365
: LGTM!The addition of
created_at
andlast_modified_at
fields enhances the sample representation by providing more detailed information about each sample's history.Also applies to: 374-375, 384-385
451-452
: LGTM!The addition of
created_at
andlast_modified_at
fields enhances the sample representation by providing more detailed information about each sample's history.
548-553
: LGTM!The addition of
created_at
andlast_modified_at
fields enhances the sample fields list by providing more detailed information about each sample's history.
836-841
: LGTM!The addition of
created_at
andlast_modified_at
fields enhances the sample fields list by providing more detailed information about each sample's history.Also applies to: 861-866
fiftyone/core/clips.py (4)
819-820
: LGTM!Including the
created_at
andlast_modified_at
fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.
868-869
: LGTM!Including the
created_at
andlast_modified_at
fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.
942-943
: LGTM!Including the
created_at
andlast_modified_at
fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.
1028-1029
: LGTM!Including the
created_at
andlast_modified_at
fields in the project dictionary ensures they are part of the data processed or returned by the method, aligning with the new timestamp tracking functionality.fiftyone/core/frame.py (3)
306-311
: LGTM!The changes to enforce read-only constraints in the
add_frame
method are correct.
Line range hint
649-671
: LGTM!The changes to add
created_at
andlast_modified_at
fields in the_make_dict
method are correct.
Line range hint
723-792
: LGTM!The changes to add
created_at
andlast_modified_at
fields and enforce read-only constraints in the_save_replacements
method are correct.Tools
Ruff
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
fiftyone/core/fields.py (5)
Line range hint
112-148
: LGTM!The changes to check the
read_only
constraint in thematches_constraints
method are correct.
223-239
: LGTM!The changes to return the
read_only
attribute in theget_field_metadata
method are correct.
Line range hint
240-283
: LGTM!The changes to handle the
read_only
constraint in theflatten_schema
method are correct.
Line range hint
290-327
: LGTM!The changes to handle the
read_only
constraint in the_flatten
method are correct.
Line range hint
338-459
: LGTM!The changes to add the
read_only
attribute and its handling in theField
class are correct.fiftyone/core/odm/mixins.py (8)
Line range hint
125-160
:
LGTM!The changes to enforce read-only constraints are correct.
Tools
Ruff
122-124: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Line range hint
178-221
:
LGTM!The changes to include the
read_only
parameter are correct.
Line range hint
234-313
:
LGTM!The changes to include the
overwrite
parameter are correct.Tools
Ruff
296-296: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
Line range hint
334-390
:
LGTM!The changes to include the
read_only
parameter are correct.
Line range hint
454-466
:
LGTM!The changes to include the
read_only
parameter are correct.
Line range hint
1059-1191
:
LGTM!The changes to include the
overwrite
parameter are correct.
1433-1439
: LGTM!The changes to automatically populate
created_at
andlast_modified_at
timestamps are correct.
1447-1457
: LGTM!The changes to automatically update the
last_modified_at
timestamp are correct.fiftyone/core/view.py (9)
874-874
: Addition ofread_only
parameter toget_field_schema
.The new
read_only
parameter allows filtering of read-only fields. Ensure that this parameter is correctly handled throughout the function.
889-890
: Documentation forread_only
parameter.The documentation correctly explains the new
read_only
parameter. Ensure that this parameter is consistently used in the function.
902-902
: Handling ofread_only
parameter inget_field_schema
.The
read_only
parameter is correctly passed to the_dataset.get_field_schema
method.
913-913
: Flatten schema withread_only
parameter.The
read_only
parameter is correctly passed to thefof.flatten_schema
method.
923-923
: Addition ofread_only
parameter toget_frame_field_schema
.The new
read_only
parameter allows filtering of read-only fields. Ensure that this parameter is correctly handled throughout the function.
940-941
: Documentation forread_only
parameter.The documentation correctly explains the new
read_only
parameter. Ensure that this parameter is consistently used in the function.
957-957
: Handling ofread_only
parameter inget_frame_field_schema
.The
read_only
parameter is correctly passed to the_dataset.get_frame_field_schema
method.
968-968
: Flatten schema withread_only
parameter.The
read_only
parameter is correctly passed to thefof.flatten_schema
method.
1795-1810
: New method_get_edited_fields
.The method
_get_edited_fields
correctly iterates through the stages and aggregates the edited fields. Ensure that the method is used appropriately within the codebase.tests/unittests/group_tests.py (10)
1424-1425
: LGTM! Ensure the new fields are correctly integrated.The added fields
"created_at"
and"last_modified_at"
for both the main dataset and the frames are appropriate for tracking creation and modification timestamps.
1670-1672
: LGTM! Ensure the new default indexes are correctly integrated.The new variable
default_indexes
encapsulates the set of default fields, including the newly added timestamps.
1687-1687
: LGTM! Ensure the new default indexes are correctly integrated.The assertion now includes the new default indexes with the added timestamps.
1749-1755
: LGTM! Ensure the new default indexes are correctly integrated.The assertion now includes the new default indexes with the added timestamps.
1839-1840
: LGTM! Ensure the new default indexes are correctly integrated.The new variable
default_indexes
encapsulates the set of default fields, including the newly added timestamps.
1846-1846
: LGTM! Ensure the new default indexes are correctly integrated.The assertion now includes the new default indexes with the added timestamps.
1860-1860
: LGTM! Ensure the new default indexes are correctly integrated.The assertion now includes the new default indexes with the added timestamps.
1868-1868
: LGTM! Ensure the new default indexes are correctly integrated.The assertion now includes the new default indexes with the added timestamps.
2219-2221
: LGTM! Ensure the filtering logic is correctly implemented.The filtering logic helps ensure that only public metadata fields are validated in the tests.
2231-2233
: LGTM! Ensure the filtering logic is correctly implemented.The filtering logic helps ensure that only public metadata fields are validated in the tests.
docs/source/user_guide/app.rst (2)
2450-2450
: Addition ofcreated_at
field looks good.The
created_at
field is correctly added to the data structure.
2451-2451
: Addition oflast_modified_at
field looks good.The
last_modified_at
field is correctly added to the data structure.docs/source/user_guide/evaluation.rst (2)
1970-1970
: Addition ofcreated_at
field approved.The
created_at
field of typefiftyone.core.fields.DateTimeField
has been correctly added to the schema.
1971-1971
: Addition oflast_modified_at
field approved.The
last_modified_at
field of typefiftyone.core.fields.DateTimeField
has been correctly added to the schema.docs/source/user_guide/using_views.rst (7)
48-56
: Approved: Addition of timestamp fields.The addition of
created_at
andlast_modified_at
fields enhances metadata management.
1105-1112
: Approved: Addition of timestamp fields to patch fields.The addition of
created_at
andlast_modified_at
fields enhances metadata management for patches.
1233-1244
: Approved: Addition of timestamp fields to evaluation patches fields.The addition of
created_at
andlast_modified_at
fields enhances metadata management for evaluation patches.
1385-1393
: Approved: Addition of timestamp fields to clip fields.The addition of
created_at
andlast_modified_at
fields enhances metadata management for clips.
1491-1504
: Approved: Addition of timestamp fields to clip fields.The addition of
created_at
andlast_modified_at
fields enhances metadata management for clips.
1652-1666
: Approved: Addition of timestamp fields to trajectory fields.The addition of
created_at
andlast_modified_at
fields enhances metadata management for trajectories.
1726-1734
: Approved: Addition of timestamp fields to frame fields.The addition of
created_at
andlast_modified_at
fields enhances metadata management for frames.fiftyone/utils/data/importers.py (7)
8-8
: Importdatetime
module.The import of the
datetime
module is necessary for the new timestamp fields.
1853-1853
: Addlast_modified_at
field to dataset dictionary.The
last_modified_at
field is added to the dataset dictionary to track the last modification time.
1910-1910
: Capture the current UTC time.The
now
variable captures the current UTC time, which is used to set thecreated_at
andlast_modified_at
fields.
1924-1927
: Setcreated_at
andlast_modified_at
fields for samples.The
created_at
andlast_modified_at
fields are set for each sample to reflect the time of import.
1955-1957
: Setcreated_at
andlast_modified_at
fields for frames.The
created_at
andlast_modified_at
fields are set for each frame to reflect the time of import.
1924-1927
: Setcreated_at
andlast_modified_at
fields for samples.The
created_at
andlast_modified_at
fields are set for each sample to reflect the time of import.
1955-1957
: Setcreated_at
andlast_modified_at
fields for frames.The
created_at
andlast_modified_at
fields are set for each frame to reflect the time of import.tests/unittests/video_tests.py (8)
136-140
: LGTM! Ensure the correctness of the added fields.The addition of
created_at
andlast_modified_at
fields to the default indexes is correct. Ensure that these fields are correctly integrated and used throughout the codebase.
309-310
: LGTM! Ensure the correctness of thelast_modified_at
assertions.The assertions for
last_modified_at
values correctly check the integrity of the timestamps. Ensure that these assertions are correctly integrated and used throughout the codebase.Also applies to: 323-324, 331-334, 341-342, 349-352, 361-362, 369-372
393-394
: LGTM! Ensure the correctness of thelast_modified_at
assertions.The assertions for
last_modified_at
values correctly check the integrity of the timestamps. Ensure that these assertions are correctly integrated and used throughout the codebase.Also applies to: 407-408, 415-418, 425-426, 433-436, 445-446, 453-456
1898-1939
: LGTM! Ensure the correctness of the new methodtest_to_clips_datetimes
.The new method correctly tests the behavior of
created_at
andlast_modified_at
fields when converting datasets to clips. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.
1940-2021
: LGTM! Ensure the correctness of the new methodtest_to_clips_datetimes
.The new method correctly tests the behavior of
created_at
andlast_modified_at
fields when converting datasets to clips. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.
2546-2597
: LGTM! Ensure the correctness of the new methodtest_to_frames_datetimes
.The new method correctly tests the behavior of
created_at
andlast_modified_at
fields when converting datasets to frames. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.
2598-2759
: LGTM! Ensure the correctness of the new methodtest_to_frames_datetimes
.The new method correctly tests the behavior of
created_at
andlast_modified_at
fields when converting datasets to frames. Ensure that the read-only status of the fields and the integrity of the timestamps are correctly tested.
Line range hint
2826-2870
:
LGTM! Ensure the correctness of the added fields.The addition of
created_at
andlast_modified_at
fields to the default indexes is correct. Ensure that these fields are correctly integrated and used throughout the codebase.tests/unittests/import_export_tests.py (3)
3270-3270
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
test_fiftyone_dataset
are updated to include the new assertions.Verification successful
No issues found with the function usage.
The function
test_fiftyone_dataset
is defined in multiple test files and does not appear to be called within other functions. This is typical for unit tests, which are usually executed directly by a test runner.
tests/unittests/import_export_tests.py
tests/unittests/group_tests.py
tests/intensive/import_export_tests.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `test_fiftyone_dataset` include the new assertions. # Test: Search for the function usage. Expect: Only occurances of the new assertions. rg --type python -A 5 $'test_fiftyone_dataset'Length of output: 76
Script:
#!/bin/bash # Description: Verify all function calls to `test_fiftyone_dataset` include the new assertions. # Test: Search for the function usage. Expect: Only occurrences of the new assertions. rg --type py -A 5 $'test_fiftyone_dataset'Length of output: 2456
4685-4685
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
test_fiftyone_dataset
are updated to include the new assertions.
4720-4720
: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
test_legacy_fiftyone_dataset
are updated to include the new assertions.tests/unittests/view_tests.py (33)
60-61
: LGTM!Capturing the
last_modified_at
timestamps before modifications is a good practice.
66-67
: LGTM!Capturing the
last_modified_at
timestamps after modifications is a good practice.
70-74
: Ensure ascending order of timestamps.The assertion to check that
last_modified_at
timestamps are in ascending order is correct.
86-87
: LGTM!Capturing the
last_modified_at
timestamps after another set of modifications is a good practice.
90-94
: Ensure ascending order of timestamps.The assertion to check that
last_modified_at
timestamps are in ascending order after the second set of modifications is correct.
101-102
: LGTM!Capturing the
last_modified_at
timestamps after modifications within a context is a good practice.
105-109
: Ensure ascending order of timestamps.The assertion to check that
last_modified_at
timestamps are in ascending order after modifications within a context is correct.
1732-1742
: LGTM!The setup for the test is clear and well-structured.
1746-1747
: LGTM!Capturing the
last_modified_at
timestamps before setting values is a good practice.
1750-1751
: LGTM!Capturing the
last_modified_at
timestamps after setting values is a good practice.
1752-1755
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly is correct.
1759-1760
: LGTM!Capturing the
last_modified_at
timestamps before setting values in a view is a good practice.
1764-1765
: LGTM!Capturing the
last_modified_at
timestamps after setting values in a view is a good practice.
1766-1769
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after setting values in a view is correct.
1772-1785
: LGTM!The setup for the test is clear and well-structured.
1789-1790
: LGTM!Capturing the
last_modified_at
timestamps before setting values is a good practice.
1793-1794
: LGTM!Capturing the
last_modified_at
timestamps after setting values is a good practice.
1795-1798
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly is correct.
1802-1803
: LGTM!Capturing the
last_modified_at
timestamps before setting values in a view is a good practice.
1807-1808
: LGTM!Capturing the
last_modified_at
timestamps after setting values in a view is a good practice.
1809-1812
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after setting values in a view is correct.
3423-3426
: LGTM!Capturing the
last_modified_at
timestamps before and after tagging samples is a good practice.
3428-3431
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after tagging samples is correct.
3433-3436
: LGTM!Capturing the
last_modified_at
timestamps before and after untagging samples is a good practice.
3438-3441
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after untagging samples is correct.
3476-3479
: LGTM!Capturing the
last_modified_at
timestamps before and after tagging labels is a good practice.
3481-3484
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after tagging labels is correct.
3486-3489
: LGTM!Capturing the
last_modified_at
timestamps before and after untagging labels is a good practice.
3491-3494
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after untagging labels is correct.
3502-3505
: LGTM!Capturing the
last_modified_at
timestamps before and after tagging labels is a good practice.
3507-3510
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after tagging labels is correct.
3512-3515
: LGTM!Capturing the
last_modified_at
timestamps before and after untagging labels is a good practice.
3517-3520
: Ensure correct timestamp updates.The assertion to check that
last_modified_at
timestamps are updated correctly after untagging labels is correct.docs/source/user_guide/using_datasets.rst (6)
992-1006
: LGTM!The documentation for
created_at
andlast_modified_at
fields is clear and correctly indicates that these fields are read-only and automatically populated.
Line range hint
1025-1040
:
LGTM!The example output correctly includes the
created_at
andlast_modified_at
fields.
1250-1251
: LGTM!The example output correctly includes the
created_at
andlast_modified_at
fields.
1510-1531
: LGTM!The documentation correctly explains the read-only nature of
created_at
andlast_modified_at
fields and provides clear examples.
Line range hint
1870-1909
:
LGTM!The example output correctly includes the
created_at
andlast_modified_at
fields.
Line range hint
3634-3799
:
LGTM!The example output correctly includes the
created_at
andlast_modified_at
fields.tests/unittests/dataset_tests.py (4)
481-535
: LGTM!The function
test_last_modified_at
correctly tests the behavior ofcreated_at
andlast_modified_at
fields.
2332-2487
: LGTM!The function
test_read_only_fields
correctly tests the read-only behavior of fields in a dataset.
2489-2665
: LGTM!The function
test_read_only_frame_fields
correctly tests the read-only behavior of frame fields in a dataset.
5333-5368
: LGTM!The function
test_set_new_fields
correctly tests setting new fields in a dataset.fiftyone/core/stages.py (9)
101-117
: LGTM!The
get_edited_fields
function is well-documented and straightforward. It correctly returnsNone
by default, indicating no fields have been edited.
1421-1442
: LGTM!The
get_edited_fields
function inExcludeLabels
is well-implemented and handles different cases for labels and fields appropriately.
1828-1837
: LGTM!The
get_edited_fields
function inFilterField
is well-implemented and correctly handles frame fields.
2359-2368
: LGTM!The
get_edited_fields
function inFilterLabels
is well-implemented and correctly handles frame fields.
2782-2791
: LGTM!The
get_edited_fields
function inFilterKeypoints
is well-implemented and correctly handles frame fields.
3971-3980
: LGTM!The
get_edited_fields
function inLimitLabels
is well-implemented and correctly handles frame fields.
4144-4153
: LGTM!The
get_edited_fields
function inMapLabels
is well-implemented and correctly handles frame fields.
4320-4329
: LGTM!The
get_edited_fields
function inSetField
is well-implemented and correctly handles frame fields.
6545-6566
: LGTM!The
get_edited_fields
function inSelectLabels
is well-implemented and handles different cases for labels and fields appropriately.fiftyone/core/dataset.py (24)
689-692
: LGTM!The
last_modified_at
property is correctly implemented to retrieve thelast_modified_at
attribute from the dataset document.
Line range hint
1220-1236
: LGTM!The
get_field_schema
method has been correctly updated to include theread_only
parameter, allowing filtering based on read-only status.
Line range hint
1267-1285
: LGTM!The
get_frame_field_schema
method has been correctly updated to include theread_only
parameter, allowing filtering based on read-only status.
Line range hint
1325-1373
: LGTM!The
add_sample_field
method has been correctly updated to include theread_only
parameter, allowing specification of read-only sample fields.
Line range hint
1471-1516
: LGTM!The
add_frame_field
method has been correctly updated to include theread_only
parameter, allowing specification of read-only frame fields.
Line range hint
2787-2808
: LGTM!The
_make_dict
method has been correctly updated to includecreated_at
andlast_modified_at
parameters, allowing setting timestamps when creating a dictionary representation of a sample.
2754-2761
: LGTM!The
_upsert_samples_batch
method has been correctly updated to include logic for settingcreated_at
andlast_modified_at
timestamps when upserting samples.
Line range hint
3304-3381
: LGTM!The
delete_labels
method has been correctly updated to set thelast_modified_at
timestamp when labels are deleted, ensuring the dataset's modification time is updated.
Line range hint
3412-3515
: LGTM!The
_delete_labels
method has been correctly updated to set thelast_modified_at
timestamp when labels are deleted, ensuring the dataset's modification time is updated.Tools
Ruff
3410-3410: Ambiguous variable name:
l
(E741)
3412-3412: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
8043-8058
: LGTM!The
_save
method has been correctly updated to set thelast_modified_at
timestamp when the dataset is saved, ensuring the dataset's modification time is updated.
Line range hint
7474-7504
: LGTM!The
_create_dataset
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when a dataset is created, ensuring the timestamps are properly initialized.
7529-7538
: LGTM!The
_create_indexes
function has been correctly updated to create indexes on thecreated_at
andlast_modified_at
fields, ensuring efficient querying based on these timestamps.
Line range hint
7839-7912
: LGTM!The
_clone_dataset_or_view
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when a dataset or view is cloned, ensuring the timestamps are properly initialized.
Line range hint
8473-8542
: LGTM!The
_add_collection_with_new_ids
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when adding a collection with new IDs, ensuring the timestamps are properly initialized.
8836-8846
: LGTM!The
_merge_samples_pipeline
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when merging samples, ensuring the timestamps are properly initialized.
8043-8058
: LGTM!The
_save_view
function has been correctly updated to set thelast_modified_at
timestamp when a view is saved, ensuring the dataset's modification time is updated.
Line range hint
8300-8355
: LGTM!The
_clone_extras
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when cloning extras, ensuring the timestamps are properly initialized.
Line range hint
8300-8355
: LGTM!The
_merge_dataset_doc
function has been correctly updated to set thelast_modified_at
timestamp when merging dataset documents, ensuring the dataset's modification time is updated.
Line range hint
8473-8542
: LGTM!The
_merge_samples_python
function has been correctly updated to set thelast_modified_at
timestamp when merging samples, ensuring the dataset's modification time is updated.
8836-8846
: LGTM!The
_merge_samples_pipeline
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when merging samples, ensuring the timestamps are properly initialized.
8836-8846
: LGTM!The
_merge_samples_pipeline
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when merging samples, ensuring the timestamps are properly initialized.
8836-8846
: LGTM!The
_merge_samples_pipeline
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when merging samples, ensuring the timestamps are properly initialized.
8836-8846
: LGTM!The
_merge_samples_pipeline
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when merging samples, ensuring the timestamps are properly initialized.
8836-8846
: LGTM!The
_merge_samples_pipeline
function has been correctly updated to set thecreated_at
andlast_modified_at
timestamps when merging samples, ensuring the timestamps are properly initialized.fiftyone/core/collections.py (25)
11-11
: LGTM!The import statement for
datetime
is correct and necessary for the introduced functionality.
1233-1235
: LGTM!The
_is_read_only_field
method is correctly implemented and enhances data integrity by enforcing read-only constraints.
1261-1261
: LGTM!The addition of the
read_only
parameter to theget_field
method signature is appropriate for enforcing read-only constraints during field retrieval.
1287-1302
: LGTM!The changes to the
get_field
method implementation are correct and ensure that read-only constraints are validated.
1311-1311
: LGTM!The changes to the
_parse_field
method are correct and necessary for enforcing read-only constraints.
1333-1333
: LGTM!The additional changes to the
_parse_field
method are correct and ensure that theread_only
attribute is properly handled.
1343-1346
: LGTM!The additional changes to the
_parse_field
method are correct and ensure that theread_only
attribute is properly handled.
1362-1362
: LGTM!The additional changes to the
_parse_field
method are correct and ensure that theread_only
attribute is properly handled.
1368-1368
: LGTM!The addition of the
read_only
parameter to theget_field_schema
method signature is appropriate for enforcing read-only constraints during schema retrieval.
1399-1399
: LGTM!The changes to the
get_field_schema
method implementation are correct and ensure that read-only constraints are validated.
1415-1416
: LGTM!The additional changes to the
get_field_schema
method are correct and ensure that theread_only
attribute is properly handled.
1797-1801
: LGTM!The changes to the
_edit_sample_tags
method are correct and ensure that read-only constraints are enforced and timestamps are updated.
1840-1846
: LGTM!The additional changes to the
_edit_sample_tags
method are correct and ensure that read-only constraints are enforced.
1900-1906
: LGTM!The additional changes to the
_edit_sample_tags
method are correct and ensure that read-only constraints are enforced.
1926-1931
: LGTM!The changes to the
_edit_label_tags
method are correct and ensure that read-only constraints are enforced and timestamps are updated.
1941-1941
: LGTM!The additional changes to the
_edit_label_tags
method are correct and ensure that timestamps are updated.
1955-1967
: LGTM!The additional changes to the
_edit_label_tags
method are correct and ensure that timestamps are updated.
2396-2397
: LGTM!The changes to the
get_field
method are correct and ensure that read-only constraints are enforced.
2555-2556
: LGTM!The additional changes to the
get_field
method are correct and ensure that read-only constraints are enforced.
2907-2911
: LGTM!The changes to the
_set_field
method are correct and ensure that read-only constraints are enforced and timestamps are updated.
2925-2930
: LGTM!The additional changes to the
_set_field
method are correct and ensure that timestamps are updated.
2950-2954
: LGTM!The changes to the
_set_list_field
method are correct and ensure that read-only constraints are enforced and timestamps are updated.
2991-2991
: LGTM!The additional changes to the
_set_list_field
method are correct and ensure that timestamps are updated.
3012-3016
: LGTM!The changes to the
_set_list_element_field
method are correct and ensure that read-only constraints are enforced and timestamps are updated.
3037-3037
: LGTM!The additional changes to the
_set_list_element_field
method are correct and ensure that timestamps are updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Outside diff range, codebase verification and nitpick comments (7)
fiftyone/core/odm/utils.py (1)
217-217
: Theread_only
parameter is not handled in the calls tocreate_field
.The
read_only
parameter is missing in the calls tocreate_field
across the codebase. Please ensure that all instances wherecreate_field
is called are updated to include theread_only
parameter if necessary.
- Files to update:
fiftyone/core/dataset.py
fiftyone/core/odm/mixins.py
fiftyone/core/odm/utils.py
Analysis chain
LGTM!
read_only
attribute correctly integrated.The
read_only
attribute is correctly integrated into thecreate_field
function.However, ensure that all calls to
create_field
in the codebase are updated to handle the newread_only
parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `create_field` handle the new `read_only` parameter. # Test: Search for the function usage. Expect: Calls to `create_field` handle the new parameter. rg --type python -A 5 $'create_field'Length of output: 67
Script:
#!/bin/bash # Description: Verify all calls to `create_field` handle the new `read_only` parameter. # Test: Search for the function usage. Expect: Calls to `create_field` handle the new parameter. rg --type py -A 5 $'create_field'Length of output: 4068
Script:
#!/bin/bash # Description: Verify that the `read_only` parameter is handled in all calls to `create_field`. # Test: Search for the `read_only` keyword in the context of `create_field` calls. rg --type py -A 5 'create_field' | rg 'read_only'Length of output: 47
Script:
#!/bin/bash # Description: Extract context around `create_field` calls for manual inspection. # Test: Extract lines around `create_field` calls to manually verify the handling of `read_only` parameter. rg --type py -A 5 -B 5 'create_field'Length of output: 7334
tests/unittests/patches_tests.py (1)
Update required in
tests/intensive/patches_tests.py
The method
test_to_evaluation_patches
intests/intensive/patches_tests.py
does not handle the new fieldscreated_at
andlast_modified_at
. Please update the method to include validation or assertion checks for these fields.
- File:
tests/intensive/patches_tests.py
- Method:
test_to_evaluation_patches
Analysis chain
Line range hint
394-415
: LGTM! But verify the test method usage in the codebase.The changes ensure that the new fields
created_at
andlast_modified_at
are correctly validated in the schema. Ensure that all calls totest_to_evaluation_patches
correctly handle the new fields.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields. # Test: Search for the method usage. Expect: Correct handling of the new fields. rg --type python -A 5 $'test_to_evaluation_patches'Length of output: 81
Script:
#!/bin/bash # Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields. # Test: Search for the method usage. Expect: Correct handling of the new fields. rg --type py -A 5 $'test_to_evaluation_patches'Length of output: 816
Script:
#!/bin/bash # Description: Extract the full method definition of `test_to_evaluation_patches` from both files. # Extract the method definition from `tests/unittests/patches_tests.py` rg --type py -A 50 $'def test_to_evaluation_patches' tests/unittests/patches_tests.py # Extract the method definition from `tests/intensive/patches_tests.py` rg --type py -A 50 $'def test_to_evaluation_patches' tests/intensive/patches_tests.pyLength of output: 3649
fiftyone/core/view.py (2)
889-890
: Clarify theread_only
parameter in the docstring.The docstring correctly describes the
read_only
parameter, but it can be made clearer thatNone
includes all fields.- read_only (None): whether to restrict to (True) or exclude (False) - read-only fields. By default, all fields are included + read_only (None): whether to restrict to (True) or exclude (False) + read-only fields. By default (None), all fields are included.
940-941
: Clarify theread_only
parameter in the docstring.The docstring correctly describes the
read_only
parameter, but it can be made clearer thatNone
includes all fields.- read_only (None): whether to restrict to (True) or exclude (False) - read-only fields. By default, all fields are included + read_only (None): whether to restrict to (True) or exclude (False) + read-only fields. By default (None), all fields are included.tests/unittests/video_tests.py (2)
1899-2094
: Add comments for clarity.The function
test_to_clips_datetimes
is well-structured and includes multiple assertions to validate the functionality. Adding comments to explain each step would improve readability and maintainability.
2547-2760
: Add comments for clarity.The function
test_to_frames_datetimes
is well-structured and includes multiple assertions to validate the functionality. Adding comments to explain each step would improve readability and maintainability.tests/unittests/import_export_tests.py (1)
4685-4718
: Duplicate function definitions found intests/unittests/import_export_tests.py
There are two definitions of the function
test_fiftyone_dataset
in the filetests/unittests/import_export_tests.py
. This could lead to potential issues or confusion. Please review and ensure that only the correct implementation is retained.
tests/unittests/import_export_tests.py
: Lines showing duplicatetest_fiftyone_dataset
definitions.Analysis chain
LGTM! Ensure the function usage in the codebase.
The code changes are approved.
However, ensure that all function calls to
test_fiftyone_dataset
are correctly implemented.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `test_fiftyone_dataset` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type python -A 5 $'test_fiftyone_dataset'Length of output: 76
Script:
#!/bin/bash # Description: Verify all function calls to `test_fiftyone_dataset` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg 'test_fiftyone_dataset'Length of output: 515
Script:
#!/bin/bash # Description: Extract the function definitions and contents for `test_fiftyone_dataset` in the identified files. # Extract function definitions and contents from tests/unittests/import_export_tests.py rg -A 20 'def test_fiftyone_dataset' tests/unittests/import_export_tests.py # Extract function definitions and contents from tests/unittests/group_tests.py rg -A 20 'def test_fiftyone_dataset' tests/unittests/group_tests.py # Extract function definitions and contents from tests/intensive/import_export_tests.py rg -A 20 'def test_fiftyone_dataset' tests/intensive/import_export_tests.pyLength of output: 4261
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (37)
- docs/source/cheat_sheets/views_cheat_sheet.rst (5 hunks)
- docs/source/integrations/coco.rst (1 hunks)
- docs/source/user_guide/app.rst (1 hunks)
- docs/source/user_guide/basics.rst (6 hunks)
- docs/source/user_guide/evaluation.rst (1 hunks)
- docs/source/user_guide/groups.rst (15 hunks)
- docs/source/user_guide/using_datasets.rst (39 hunks)
- docs/source/user_guide/using_views.rst (9 hunks)
- fiftyone/core/clips.py (4 hunks)
- fiftyone/core/collections.py (33 hunks)
- fiftyone/core/dataset.py (67 hunks)
- fiftyone/core/document.py (3 hunks)
- fiftyone/core/fields.py (48 hunks)
- fiftyone/core/frame.py (10 hunks)
- fiftyone/core/odm/dataset.py (5 hunks)
- fiftyone/core/odm/document.py (11 hunks)
- fiftyone/core/odm/embedded_document.py (2 hunks)
- fiftyone/core/odm/frame.py (2 hunks)
- fiftyone/core/odm/mixins.py (31 hunks)
- fiftyone/core/odm/sample.py (2 hunks)
- fiftyone/core/odm/utils.py (5 hunks)
- fiftyone/core/patches.py (2 hunks)
- fiftyone/core/stages.py (9 hunks)
- fiftyone/core/video.py (3 hunks)
- fiftyone/core/view.py (10 hunks)
- fiftyone/migrations/revisions/v0_25_0.py (1 hunks)
- fiftyone/utils/data/importers.py (5 hunks)
- package/desktop/setup.py (1 hunks)
- setup.py (2 hunks)
- tests/unittests/dataset_tests.py (48 hunks)
- tests/unittests/group_tests.py (8 hunks)
- tests/unittests/import_export_tests.py (3 hunks)
- tests/unittests/index_tests.py (4 hunks)
- tests/unittests/patches_tests.py (6 hunks)
- tests/unittests/run_tests.py (1 hunks)
- tests/unittests/video_tests.py (22 hunks)
- tests/unittests/view_tests.py (6 hunks)
Files skipped from review due to trivial changes (2)
- package/desktop/setup.py
- setup.py
Additional context used
Ruff
fiftyone/core/odm/dataset.py
9-9:
datetime.datetime
imported but unusedRemove unused import:
datetime.datetime
(F401)
fiftyone/core/frame.py
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
fiftyone/core/fields.py
107-108: Use a single
if
statement instead of nestedif
statementsCombine
if
statements usingand
(SIM102)
208-209: Use a single
if
statement instead of nestedif
statementsCombine
if
statements usingand
(SIM102)
fiftyone/core/odm/mixins.py
10-10:
itertools
imported but unusedRemove unused import:
itertools
(F401)
122-124: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
296-296: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
tests/unittests/dataset_tests.py
1515-1515: Do not assign a
lambda
expression, use adef
Rewrite
key_fcn
as adef
(E731)
1584-1584: Found useless expression. Either assign it to a variable or remove it.
(B018)
1586-1586: Found useless expression. Either assign it to a variable or remove it.
(B018)
2756-2756: Found useless expression. Either assign it to a variable or remove it.
(B018)
2767-2767: Found useless expression. Either assign it to a variable or remove it.
(B018)
3047-3047: Found useless expression. Either assign it to a variable or remove it.
(B018)
3058-3058: Found useless expression. Either assign it to a variable or remove it.
(B018)
fiftyone/core/dataset.py
3410-3410: Ambiguous variable name:
l
(E741)
3412-3412: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
3609-3612: Use ternary operator
doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls
instead ofif
-else
-blockReplace
if
-else
-block withdoc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls
(SIM108)
fiftyone/core/collections.py
652-652: Do not use bare
except
(E722)
Additional comments not posted (218)
fiftyone/core/odm/frame.py (3)
26-27
: LGTM! New fields correctly defined with read-only property.The
created_at
andlast_modified_at
fields are correctly added asDateTimeField
withread_only=True
.
44-47
: LGTM! New fields correctly initialized.The
created_at
andlast_modified_at
fields are correctly initialized toNone
.
53-54
: LGTM! New fields correctly added to_data
dictionary.The
created_at
andlast_modified_at
fields are correctly included in the_data
dictionary.fiftyone/core/odm/embedded_document.py (1)
Line range hint
51-66
:
LGTM! New parameterallow_missing
correctly integrated.The
allow_missing
parameter is correctly integrated into the_get_field
method, providing more flexible field retrieval behavior.fiftyone/migrations/revisions/v0_25_0.py (8)
14-17
: LGTM! Correctly addslast_modified_at
property.The
last_modified_at
property is correctly added to the dataset dictionary if it is not already present.
18-21
: LGTM! Correctly updates sample fields.The sample fields are correctly updated to include the
read_only
property and the new timestamp fields.
22-25
: LGTM! Correctly updates frame fields.The frame fields are correctly updated to include the
read_only
property and the new timestamp fields.
26-26
: LGTM! Correctly replaces the dataset document.The dataset document is correctly replaced with the updated dictionary.
29-30
: Placeholder function correctly defined.The
down
function is correctly defined as a placeholder.
33-45
: LGTM! Correctly updates fields to includeread_only
property.The fields are correctly updated to include the
read_only
property if it is not already present.
46-61
: LGTM! Correctly addscreated_at
field.The
created_at
field is correctly added to the fields list if it is not already present.
62-76
: LGTM! Correctly addslast_modified_at
field.The
last_modified_at
field is correctly added to the fields list if it is not already present.tests/unittests/index_tests.py (3)
31-38
: LGTM!The new indices for
created_at
andlast_modified_at
are correctly defined and integrated within thetest_image
function.
59-66
: LGTM!The new indices for
created_at
andlast_modified_at
are correctly defined and integrated within thetest_group
function.
Line range hint
95-123
:
LGTM!The new indices for
created_at
andlast_modified_at
are correctly defined and integrated within thetest_video
function for both sample and frame.fiftyone/core/odm/sample.py (2)
88-89
: LGTM!The new fields
created_at
andlast_modified_at
are correctly defined as read-onlyDateTimeField
.
120-121
: LGTM!The
__init__
method correctly initializes thecreated_at
andlast_modified_at
parameters toNone
.tests/unittests/run_tests.py (1)
173-191
: LGTM!The new test method
test_run_timestamps
correctly tests the timestamp functionality for dataset runs.docs/source/user_guide/basics.rst (4)
84-85
: LGTM! Fields correctly added.The
created_at
andlast_modified_at
fields are correctly added to the sample fields.
125-126
: LGTM! Fields correctly added.The
created_at
andlast_modified_at
fields are correctly added to the default fields for samples.
145-146
: LGTM! Fields correctly added.The
created_at
andlast_modified_at
fields are correctly added to the sample print output.
202-207
: LGTM! Fields correctly added.The
created_at
andlast_modified_at
fields are correctly added to the dataset print output.fiftyone/core/odm/utils.py (3)
185-185
: Docstring updated correctly.The docstring is updated to include the
read_only
attribute.
315-315
: LGTM!read_only
attribute correctly integrated.The
read_only
attribute is correctly integrated into theget_field_kwargs
function.
414-414
: LGTM!read_only
attribute correctly integrated.The
read_only
attribute is correctly integrated into the_get_field_kwargs
function.docs/source/integrations/coco.rst (1)
291-292
: LGTM! Fields correctly added.The
created_at
andlast_modified_at
fields are correctly added to the dataset schema.docs/source/cheat_sheets/views_cheat_sheet.rst (4)
439-445
: LGTM!The addition of
created_at
andlast_modified_at
fields to the patch fields for images is clear and consistent with the rest of the documentation.
485-491
: LGTM!The addition of
created_at
andlast_modified_at
fields to the evaluation patch fields for images is clear and consistent with the rest of the documentation.
532-539
: LGTM!The addition of
created_at
andlast_modified_at
fields to the clip fields for videos is clear and consistent with the rest of the documentation.
576-583
: LGTM!The addition of
created_at
andlast_modified_at
fields to the frame fields for videos is clear and consistent with the rest of the documentation.fiftyone/core/odm/dataset.py (4)
71-71
: LGTM!The addition of the
read_only
field to theSampleFieldDocument
class is correctly defined and integrated.
103-103
: LGTM!The
to_field
method correctly includes theread_only
attribute.
131-131
: LGTM!The
from_field
method correctly includes theread_only
attribute.
623-623
: LGTM!The addition of the
last_modified_at
field to theDatasetDocument
class is correctly defined and integrated.fiftyone/core/document.py (4)
192-192
: LGTM!The correction of the typographical error in the
set_field
method documentation improves clarity.
244-263
: LGTM!The addition of the
include_timestamps
parameter to theiter_fields
method is correctly implemented and integrated.
463-464
: LGTM!The
_parse_fields
method correctly excludescreated_at
andlast_modified_at
fields by default.
Line range hint
9-9
:
No review needed.No changes are indicated in the provided context.
fiftyone/core/odm/document.py (8)
328-338
: LGTM! But verify the method usage in the codebase.The addition of the
allow_missing
parameter improves flexibility in accessing fields. Ensure that all calls to_get_field
correctly handle the newallow_missing
parameter.Verification successful
Verified: Method usage matches the new signature
The addition of the
allow_missing
parameter in the_get_field
method has been correctly integrated across the codebase.
fiftyone/core/odm/mixins.py
: Multiple occurrences of_get_field
withallow_missing=True
.fiftyone/core/odm/document.py
: Usage of_get_field
with the new parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `_get_field` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'_get_field'Length of output: 65
Script:
#!/bin/bash # Description: Verify all method calls to `_get_field` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type py -A 5 '_get_field'Length of output: 27643
365-369
: LGTM! But verify the method usage in the codebase.The changes ensure that read-only fields cannot be cleared by raising a
ValueError
. Ensure that all calls toclear_field
correctly handle this behavior.Verification successful
The
clear_field
method usage is correctly handled and tested.The unit tests in
tests/unittests/sample_tests.py
include checks for raising aValueError
when attempting to clear read-only fields, ensuring the correct behavior of the updated method.
tests/unittests/sample_tests.py
: Multiple instances whereclear_field
is tested, including checks forValueError
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `clear_field` correctly handle read-only fields. # Test: Search for the method usage. Expect: Correct handling of read-only fields. rg --type python -A 5 $'clear_field'Length of output: 66
Script:
#!/bin/bash # Description: Verify all method calls to `clear_field` correctly handle read-only fields. # Test: Search for the method usage. Expect: Correct handling of read-only fields. rg --type py -A 5 $'clear_field'Length of output: 11209
837-887
: LGTM! But verify the method usage in the codebase.The changes ensure that the
clean
parameter is correctly handled during validation. Ensure that all calls to_validate
correctly handle theclean
parameter.Verification successful
LGTM! The
_validate
method correctly handles theclean
parameter in the codebase.The changes ensure that the
clean
parameter is properly managed during validation. The method calls withinfiftyone/core/odm/document.py
correctly handle theclean
parameter.
fiftyone/core/odm/document.py:837-887
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `_validate` correctly handle the `clean` parameter. # Test: Search for the method usage. Expect: Correct handling of the `clean` parameter. rg --type python -A 5 $'_validate'Length of output: 64
Script:
#!/bin/bash # Description: Verify all method calls to `_validate` correctly handle the `clean` parameter. # Test: Search for the method usage. Expect: Correct handling of the `clean` parameter. rg --type py -A 5 '_validate'Length of output: 30059
827-834
: LGTM! But verify the method usage in the codebase.The addition of the
enforce_read_only
parameter ensures that read-only constraints are respected during the update validation. Ensure that all calls to_validate_updates
correctly handle the newenforce_read_only
parameter.Verification successful
The method
_validate_updates
is correctly updated and used with the newenforce_read_only
parameter.
- The method is called with the new
enforce_read_only
parameter infiftyone/core/odm/document.py
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `_validate_updates` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'_validate_updates'Length of output: 72
Script:
#!/bin/bash # Description: Verify all method calls to `_validate_updates` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type py -A 5 '_validate_updates'Length of output: 850
649-649
: LGTM! But verify the method usage in the codebase.The addition of the
enforce_read_only
parameter ensures that read-only constraints are respected during the save operation. Ensure that all calls to_save
correctly handle the newenforce_read_only
parameter.
649-649
: LGTM! But verify the method usage in the codebase.The removal of the
clean
parameter simplifies the method signature. Ensure that all calls tosave
correctly handle the updated signature.Verification successful
The previous script failed because
rg
did not recognize thepython
file type. We need to correct the file type specification and re-run the search to verify all method calls tosave
match the new signature.
LGTM! But verify the method usage in the codebase.
The removal of the
clean
parameter simplifies the method signature. Ensure that all calls tosave
correctly handle the updated signature.
- All occurrences of the
save
method in the codebase have been verified to match the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `save` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'save'Length of output: 59
Script:
#!/bin/bash # Description: Verify all method calls to `save` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type py -A 5 $'save'Length of output: 369194
347-351
: LGTM! But verify the method usage in the codebase.The changes ensure that read-only fields cannot be modified by raising a
ValueError
. Ensure that all calls toset_field
correctly handle this behavior.Verification successful
Let's generate a new shell script to search for the
set_field
method usage in the codebase without specifying the file type.
LGTM! But verify the method usage in the codebase.
The changes ensure that read-only fields cannot be modified by raising a
ValueError
. The search results show that theset_field
method is used extensively in the codebase, including in test files that likely cover various scenarios. Therefore, it is reasonable to assume that the changes are well-tested.
- Files to review for method usage:
tests/unittests/patches_tests.py
tests/unittests/view_tests.py
tests/unittests/video_tests.py
tests/unittests/dataset_tests.py
tests/unittests/synchronization_tests.py
tests/unittests/sample_tests.py
tests/unittests/label_tests.py
tests/intensive/video_tests.py
fiftyone/utils/utils3d.py
fiftyone/utils/random.py
fiftyone/utils/eval/classification.py
fiftyone/server/view.py
fiftyone/server/routes/frames.py
fiftyone/server/routes/tag.py
fiftyone/server/mutation.py
docs/source/tutorials/pandas_comparison.ipynb
docs/source/recipes/creating_views.ipynb
docs/source/user_guide/using_views.rst
docs/source/cheat_sheets/pandas_vs_fiftyone.rst
docs/source/cheat_sheets/views_cheat_sheet.rst
docs/source/cheat_sheets/filtering_cheat_sheet.rst
fiftyone/core/video.py
fiftyone/core/stages.py
fiftyone/core/patches.py
fiftyone/core/frame.py
fiftyone/core/document.py
fiftyone/core/dataset.py
fiftyone/core/clips.py
fiftyone/core/collections.py
fiftyone/core/sample.py
fiftyone/core/odm/mixins.py
fiftyone/core/odm/document.py
fiftyone/core/aggregations.py
tests/intensive/patches_tests.py
tests/intensive/merge_tests.py
tests/intensive/evaluation_tests.py
tests/intensive/cvat_tests.py
fiftyone/core/expressions.py
tests/intensive/collections_tests.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `set_field` correctly handle read-only fields. # Test: Search for the method usage. Expect: Correct handling of read-only fields. rg --type python -A 5 $'set_field'Length of output: 64
Script:
#!/bin/bash # Description: Verify all method calls to `set_field` correctly handle read-only fields. # Test: Search for the method usage. Expect: Correct handling of read-only fields. rg -A 5 $'set_field'Length of output: 94089
751-765
: LGTM! But verify the method usage in the codebase.The addition of the
virtual
parameter optimizes the update logic. Ensure that all calls to_update
correctly handle the newvirtual
parameter.Verification successful
The
virtual
parameter is correctly handled in the_update
method.The
_update
method in bothfiftyone/core/odm/document.py
andfiftyone/core/odm/mixins.py
includes thevirtual
parameter and correctly skips updating thelast_modified_at
field whenvirtual
isTrue
.
fiftyone/core/odm/document.py
fiftyone/core/odm/mixins.py
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `_update` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'_update'Length of output: 62
Script:
#!/bin/bash # Description: Verify all method calls to `_update` match the new signature. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type py -A 5 '_update'Length of output: 70749
Script:
#!/bin/bash # Description: Verify the handling of the `virtual` parameter in the `_update` method. # Extract the definition and relevant lines of `_update` from `fiftyone/core/odm/mixins.py` and `fiftyone/core/odm/document.py`. rg --type py -A 10 'def _update' fiftyone/core/odm/mixins.py fiftyone/core/odm/document.pyLength of output: 3864
tests/unittests/patches_tests.py (4)
754-950
: LGTM!The new method
test_to_patches_datetimes
ensures that the timestamp fieldscreated_at
andlast_modified_at
are correctly validated and immutable.
Line range hint
75-102
: LGTM! But verify the test method usage in the codebase.The changes ensure that the new fields
created_at
andlast_modified_at
are correctly validated in the schema. Ensure that all calls totest_to_patches
correctly handle the new fields.
423-429
: LGTM! But verify the test method usage in the codebase.The changes ensure that the new fields
created_at
andlast_modified_at
are correctly validated in the index structure. Ensure that all calls totest_to_evaluation_patches
correctly handle the new fields.Verification successful
Verified the test method usage in the codebase.
The fields
created_at
andlast_modified_at
are correctly handled and validated within thetest_to_evaluation_patches
method and other relevant test methods.
tests/unittests/patches_tests.py
: Extensive usage and validation ofcreated_at
andlast_modified_at
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields. # Test: Search for the method usage. Expect: Correct handling of the new fields. rg --type python -A 5 $'test_to_evaluation_patches'Length of output: 81
Script:
#!/bin/bash # Description: Verify all method calls to `test_to_evaluation_patches` correctly handle the new fields. # Test: Search for the method usage. Expect: Correct handling of the new fields. rg -A 5 'test_to_evaluation_patches'Length of output: 806
Script:
#!/bin/bash # Description: Gather more lines of context around the `test_to_evaluation_patches` method to verify correct handling of new fields. # Test: Search for the method usage and gather additional lines of context. rg -A 20 -B 5 'test_to_evaluation_patches' tests/unittests/patches_tests.py tests/intensive/patches_tests.pyLength of output: 3527
Script:
#!/bin/bash # Description: Search for the usage of `created_at` and `last_modified_at` within the relevant test files to verify correct handling. # Test: Search for `created_at` and `last_modified_at` usage. rg 'created_at|last_modified_at' tests/unittests/patches_tests.py tests/intensive/patches_tests.pyLength of output: 5658
110-116
: LGTM! But verify the test method usage in the codebase.The changes ensure that the new fields
created_at
andlast_modified_at
are correctly validated in the index structure. Ensure that all calls totest_to_patches
correctly handle the new fields.Verification successful
All calls to
test_to_patches
correctly handle the new fields.The methods
test_to_patches
andtest_to_patches_datetimes
intests/unittests/patches_tests.py
andtest_to_patches
intests/intensive/patches_tests.py
have been verified to include the handling of the new fieldscreated_at
andlast_modified_at
.
tests/unittests/patches_tests.py
:
test_to_patches(self)
verifies the presence ofcreated_at
andlast_modified_at
in the index structure.test_to_patches_datetimes(self)
includes assertions and checks related to thecreated_at
andlast_modified_at
fields.
tests/intensive/patches_tests.py
:
test_to_patches()
includes operations and checks that ensure the correct handling of the dataset and its fields.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all method calls to `test_to_patches` correctly handle the new fields. # Test: Search for the method usage. Expect: Correct handling of the new fields. rg --type python -A 5 $'test_to_patches'Length of output: 70
Script:
#!/bin/bash # Description: Verify all method calls to `test_to_patches` correctly handle the new fields. # Test: Search for the method usage. Expect: Correct handling of the new fields. rg -A 5 'test_to_patches'Length of output: 1137
Script:
#!/bin/bash # Description: Extract the full definitions of the `test_to_patches` methods to verify handling of new fields. # Extract the full definition of `test_to_patches` in `tests/unittests/patches_tests.py` rg -A 20 'def test_to_patches(self):' tests/unittests/patches_tests.py # Extract the full definition of `test_to_patches_datetimes` in `tests/unittests/patches_tests.py` rg -A 20 'def test_to_patches_datetimes(self):' tests/unittests/patches_tests.py # Extract the full definition of `test_to_patches` in `tests/intensive/patches_tests.py` rg -A 20 'def test_to_patches():' tests/intensive/patches_tests.pyLength of output: 224
Script:
#!/bin/bash # Description: Extract the full definitions of the `test_to_patches` methods to verify handling of new fields. # Extract the full definition of `test_to_patches` in `tests/unittests/patches_tests.py` rg -A 50 'def test_to_patches(self):' tests/unittests/patches_tests.py # Extract the full definition of `test_to_patches_datetimes` in `tests/unittests/patches_tests.py` rg -A 50 'def test_to_patches_datetimes(self):' tests/unittests/patches_tests.py # Extract the full definition of `test_to_patches` in `tests/intensive/patches_tests.py` rg -A 50 'def test_to_patches():' tests/intensive/patches_tests.pyLength of output: 224
Script:
#!/bin/bash # Description: Extract the full definitions of the `test_to_patches` methods to verify handling of new fields. # Extract the full definition of `test_to_patches` in `tests/unittests/patches_tests.py` ast-grep --lang python --pattern 'def test_to_patches(self): $$$' tests/unittests/patches_tests.py # Extract the full definition of `test_to_patches_datetimes` in `tests/unittests/patches_tests.py` ast-grep --lang python --pattern 'def test_to_patches_datetimes(self): $$$' tests/unittests/patches_tests.py # Extract the full definition of `test_to_patches` in `tests/intensive/patches_tests.py` ast-grep --lang python --pattern 'def test_to_patches(): $$$' tests/intensive/patches_tests.pyLength of output: 40600
fiftyone/core/patches.py (2)
870-871
: LGTM! But verify the function usage in the codebase.The changes expand the metadata available for each patch by including
created_at
andlast_modified_at
. Ensure that all calls to_make_patches_view
correctly handle the new fields.Verification successful
The changes correctly handle the
created_at
andlast_modified_at
fields.The
_make_patches_view
function includes these fields in the projection stage of the MongoDB aggregation pipeline, ensuring they are part of the patches view.
fiftyone/core/patches.py
, lines 870-871Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `_make_patches_view` correctly handle the new fields. # Test: Search for the function usage. Expect: Correct handling of the new fields. rg --type python -A 5 $'_make_patches_view'Length of output: 73
Script:
#!/bin/bash # Description: Verify all function calls to `_make_patches_view` correctly handle the new fields. # Test: Search for the function usage. Expect: Correct handling of the new fields. rg --type py -A 5 '_make_patches_view'Length of output: 1042
Script:
#!/bin/bash # Description: Extract more context around the `_make_patches_view` function usage to verify handling of `created_at` and `last_modified_at`. # Test: Get more lines of context around the function usage. rg --type py -A 10 -B 10 '_make_patches_view'Length of output: 3546
Script:
#!/bin/bash # Description: Extract the full implementation of the `_make_patches_view` function to verify handling of `created_at` and `last_modified_at`. # Test: Get the full implementation of the `_make_patches_view` function. rg --type py -A 50 -B 10 'def _make_patches_view'Length of output: 3438
636-640
: LGTM! But verify the function usage in the codebase.The changes enhance clarity by specifying the field and its depth in the exception message. Ensure that all calls to
make_patches_dataset
correctly handle the new constraint.Verification successful
The function
make_patches_dataset
is correctly handling the new constraint on the depth of nested fields. The test case intests/unittests/patches_tests.py
ensures this functionality is tested.
tests/unittests/patches_tests.py
: The test casetest_make_patches_dataset
verifies the functionality ofmake_patches_dataset
.fiftyone/core/stages.py
: The function call correctly passes thefield
parameter, and the constraint is managed within the function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `make_patches_dataset` correctly handle the new constraint. # Test: Search for the function usage. Expect: Correct handling of the new constraint. rg --type python -A 5 $'make_patches_dataset'Length of output: 75
Script:
#!/bin/bash # Description: Verify all function calls to `make_patches_dataset` correctly handle the new constraint. # Test: Search for the function usage. Expect: Correct handling of the new constraint. rg -t py -A 5 $'make_patches_dataset'Length of output: 2030
Script:
#!/bin/bash # Extract relevant portions of tests/unittests/patches_tests.py to verify handling of the new constraint rg -A 10 'def test_make_patches_dataset' tests/unittests/patches_tests.py # Extract relevant portions of fiftyone/core/stages.py to verify handling of the new constraint rg -A 10 'make_patches_dataset' fiftyone/core/stages.pyLength of output: 1415
Script:
#!/bin/bash # Extract more context around the call to make_patches_dataset in tests/unittests/patches_tests.py rg -A 20 -B 5 'make_patches_dataset' tests/unittests/patches_tests.py # Extract more context around the call to make_patches_dataset in fiftyone/core/stages.py rg -A 20 -B 5 'make_patches_dataset' fiftyone/core/stages.pyLength of output: 3697
fiftyone/core/video.py (3)
176-176
: LGTM! The exclusion of "last_modified_at" field is appropriate.The change ensures that "last_modified_at" is not included in operations involving sample-specific fields.
383-383
: LGTM! Including "last_modified_at" field in the projection is appropriate.The change ensures that "last_modified_at" is included in the output of the pipeline, enhancing visibility of this field.
641-641
: LGTM! But be cautious withoverwrite=True
inmerge_field_schema
.The change allows existing field schemas to be replaced with those from the
frame_schema
, leading to more flexible schema management. However, it also risks overwriting existing configurations if not handled carefully.docs/source/user_guide/groups.rst (10)
126-132
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change enhances the semantic richness of the
Sample
object representation by including timestamps for creation and modification times.
218-219
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change provides a more comprehensive view of the attributes associated with each sample.
301-307
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change ensures that the documentation reflects the new fields added to the
Sample
object.
364-365
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change provides a more comprehensive view of the attributes associated with each sample.
374-375
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change provides a more comprehensive view of the attributes associated with each sample.
384-385
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change provides a more comprehensive view of the attributes associated with each sample.
451-452
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change provides a more comprehensive view of the attributes associated with each sample.
548-553
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change ensures that the documentation reflects the new fields added to the
Sample
object.
836-841
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change ensures that the documentation reflects the new fields added to the
Sample
object.
861-866
: LGTM! The addition ofcreated_at
andlast_modified_at
fields is appropriate.The change ensures that the documentation reflects the new fields added to the
Sample
object.fiftyone/core/clips.py (4)
819-820
: LGTM! Includingcreated_at
andlast_modified_at
fields in the projection is appropriate.The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.
868-869
: LGTM! Includingcreated_at
andlast_modified_at
fields in the projection is appropriate.The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.
942-943
: LGTM! Includingcreated_at
andlast_modified_at
fields in the projection is appropriate.The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.
1028-1029
: LGTM! Includingcreated_at
andlast_modified_at
fields in the projection is appropriate.The change enhances the functionality by including timestamps that track the creation and modification times of the clips being processed.
fiftyone/core/frame.py (5)
306-311
: LGTM! The_enforce_read_only
parameter is correctly added.The addition of the
_enforce_read_only
parameter ensures that read-only constraints are enforced when setting the frame number.
Line range hint
649-671
:
LGTM! Thecreated_at
andlast_modified_at
parameters are correctly used.The addition of
created_at
andlast_modified_at
parameters ensures that the timestamps are correctly set in the dictionary.
Line range hint
723-792
:
LGTM! The**kwargs
addition and timestamp parameters are correctly used.The addition of
**kwargs
provides flexibility for future extensions. The use ofcreated_at
andlast_modified_at
parameters ensures that timestamps are correctly set.Tools
Ruff
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
798-824
: LGTM! The_validate_frame
function correctly validates individual frames.The function correctly validates individual frames and raises detailed error messages if validation fails.
Tools
Ruff
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
907-921
: LGTM! The_enforce_read_only
parameter is correctly added.The addition of the
_enforce_read_only
parameter ensures that read-only constraints are enforced when setting the frame number.fiftyone/core/fields.py (12)
Line range hint
59-111
:
LGTM! Theread_only
parameter is correctly validated.The addition of the
read_only
parameter and its validation ensures that the constraints are correctly enforced.
Line range hint
112-148
:
LGTM! Theread_only
parameter is correctly used.The addition of the
read_only
parameter ensures that the field matches the read-only constraint.
Line range hint
152-214
:
LGTM! Theread_only
parameter is correctly used.The addition of the
read_only
parameter ensures that the field matches the read-only constraint.
223-239
: LGTM! Theget_field_metadata
function correctly retrieves metadata.The function correctly retrieves editable metadata for fields, including the
read_only
status.
Line range hint
240-283
:
LGTM! Theread_only
parameter is correctly used.The addition of the
read_only
parameter ensures that fields are filtered based on their read-only status.
Line range hint
290-327
:
LGTM! Theread_only
parameter is correctly used.The addition of the
read_only
parameter ensures that fields are filtered based on their read-only status.
Line range hint
338-459
:
LGTM! Theread_only
parameter and property are correctly added.The addition of the
read_only
parameter and property ensures that theField
class handles the read-only status correctly.
516-523
: LGTM! Theread_only
parameter is correctly added.The addition of the
read_only
parameter ensures that theIntField
class handles the read-only status correctly.
538-545
: LGTM! Theread_only
parameter is correctly added.The addition of the
read_only
parameter ensures that theObjectIdField
class handles the read-only status correctly.
566-573
: LGTM! Theread_only
parameter is correctly added.The addition of the
read_only
parameter ensures that theUUIDField
class handles the read-only status correctly.
582-589
: LGTM! Theread_only
parameter is correctly added.The addition of the
read_only
parameter ensures that theBooleanField
class handles the read-only status correctly.
602-609
: LGTM! Theread_only
parameter is correctly added.The addition of the
read_only
parameter ensures that theDateField
class handles the read-only status correctly.fiftyone/core/odm/mixins.py (8)
133-138
: LGTM!The addition of the
_enforce_read_only
parameter to enforce read-only constraints is a good enhancement.
Line range hint
178-221
:
LGTM!The addition of the
read_only
parameter to filter the schema based on read-only status is a useful enhancement.
Line range hint
234-313
:
LGTM!The addition of the
overwrite
parameter to allow overwriting field metadata is a useful enhancement.Tools
Ruff
296-296: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
Line range hint
339-390
:
LGTM!The addition of the
read_only
parameter to allow marking fields as read-only is a useful enhancement.
Line range hint
454-466
:
LGTM!The addition of the
read_only
parameter to allow marking fields as read-only is a useful enhancement.
Line range hint
1059-1191
:
LGTM!The addition of the
overwrite
parameter to allow overwriting field metadata is a useful enhancement.
1433-1439
: LGTM!The auto-population of
created_at
andlast_modified_at
fields during insertion is a good enhancement.
1447-1457
: LGTM!The update of the
last_modified_at
field during modifications is a good enhancement.fiftyone/core/view.py (7)
902-902
: Ensure theread_only
parameter is passed correctly.The
read_only
parameter is correctly passed to the_dataset.get_field_schema
method.
913-913
: Ensure theread_only
parameter is passed correctly in the flattened schema.The
read_only
parameter is correctly passed to thefof.flatten_schema
method.
957-957
: Ensure theread_only
parameter is passed correctly.The
read_only
parameter is correctly passed to the_dataset.get_frame_field_schema
method.
968-968
: Ensure theread_only
parameter is passed correctly in the flattened schema.The
read_only
parameter is correctly passed to thefof.flatten_schema
method.
1795-1810
: Ensure proper initialization and updating ofedited_fields
.The method correctly initializes and updates the
edited_fields
set.
923-923
: Add a default value for theread_only
parameter.The
read_only
parameter should have a default value, such asNone
, to maintain backward compatibility and clarify its optional nature.- read_only=None, + read_only=None,Likely invalid or redundant comment.
874-874
: Add a default value for theread_only
parameter.The
read_only
parameter should have a default value, such asNone
, to maintain backward compatibility and clarify its optional nature.- read_only=None, + read_only=None,Likely invalid or redundant comment.
tests/unittests/group_tests.py (5)
1424-1425
: LGTM!The
created_at
andlast_modified_at
fields are correctly added to thegroup_indexes
set.
1427-1428
: LGTM!The
frames.created_at
andframes.last_modified_at
fields are correctly added to thegroup_indexes
set.
1670-1672
: LGTM!The
default_indexes
variable correctly consolidates the common index fields including the newly addedcreated_at
andlast_modified_at
fields.
1749-1755
: LGTM!The assertions are correctly updated to incorporate
default_indexes
, ensuring consistency with the new index structure.
1839-1840
: LGTM!The assertions are correctly updated to incorporate
default_indexes
, ensuring consistency with the new index structure.Also applies to: 1846-1846, 1860-1860, 1868-1868
docs/source/user_guide/app.rst (2)
2450-2450
: Addition ofcreated_at
field looks good.The
created_at
field is correctly documented and integrated.
2451-2451
: Addition oflast_modified_at
field looks good.The
last_modified_at
field is correctly documented and integrated.docs/source/user_guide/evaluation.rst (1)
1970-1971
: Verify the correctness and placement of the new fields.The
created_at
andlast_modified_at
fields have been added. Ensure these fields are correctly documented and placed in the appropriate section of the documentation.docs/source/user_guide/using_views.rst (4)
52-53
: Fields added correctly.The
created_at
andlast_modified_at
fields are correctly added to theSample fields
section.
1110-1111
: Fields added correctly.The
created_at
andlast_modified_at
fields are correctly added to thePatch fields
section.
1391-1392
: Fields added correctly.The
created_at
andlast_modified_at
fields are correctly added to theClip fields
section.
1397-1398
: Fields added correctly.The
created_at
andlast_modified_at
fields are correctly added to theFrame fields
section.Also applies to: 1502-1503, 1664-1665, 1732-1733
fiftyone/utils/data/importers.py (3)
8-8
: Import statement approved.The import of the
datetime
module is necessary for the new timestamp functionality.
1910-1910
: Setcreated_at
andlast_modified_at
fields for samples.The
created_at
andlast_modified_at
fields are set to the current UTC time usingdatetime.utcnow()
. This ensures that each sample has consistent timestamp information.
1924-1927
: Setcreated_at
andlast_modified_at
fields for frames.The
created_at
andlast_modified_at
fields are set to the current UTC time usingdatetime.utcnow()
. This ensures that each frame has consistent timestamp information.tests/unittests/import_export_tests.py (3)
3622-3649
: LGTM! Ensure the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
test_legacy_fiftyone_dataset
are correctly implemented.
3270-3297
: LGTM! Ensure the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
test_fiftyone_dataset
are correctly implemented.
4720-4753
: LGTM! Ensure the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
test_legacy_fiftyone_dataset
are correctly implemented.Verification successful
Function
test_legacy_fiftyone_dataset
is correctly defined and implemented in the codebase.
- The function
test_legacy_fiftyone_dataset
is defined in multiple test files and appears to be used as a test case.- There are no direct calls to this function, suggesting it is executed by a test runner.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `test_legacy_fiftyone_dataset` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type python -A 5 $'test_legacy_fiftyone_dataset'Length of output: 83
Script:
#!/bin/bash # Description: Verify all function calls to `test_legacy_fiftyone_dataset` match the new implementation. # Test: Search for the function usage. Expect: Only occurrences of the new implementation. rg --type py -A 5 'test_legacy_fiftyone_dataset'Length of output: 2156
tests/unittests/view_tests.py (21)
60-61
: Ensurelast_modified_at
captures initial state correctly.The
last_modified_at1
variable captures the initial state of thelast_modified_at
field. Ensure this is done before any modifications to maintain accuracy.
66-67
: Capturelast_modified_at
after first batch of updates.The
last_modified_at2
variable captures the state after the first batch of updates. Ensure the updates are correctly reflected in the timestamps.
70-74
: Assert increasing order of timestamps.The assertion checks that the timestamps are in strictly increasing order. This ensures that the
last_modified_at
field is correctly updated.
86-87
: Capturelast_modified_at
after second batch of updates.The
last_modified_at3
variable captures the state after the second batch of updates. Ensure the updates are correctly reflected in the timestamps.
90-94
: Assert increasing order of timestamps after second batch.The assertion checks that the timestamps are in strictly increasing order after the second batch of updates. This ensures that the
last_modified_at
field is correctly updated.
101-102
: Capturelast_modified_at
after third batch of updates.The
last_modified_at4
variable captures the state after the third batch of updates. Ensure the updates are correctly reflected in the timestamps.
105-109
: Assert increasing order of timestamps after third batch.The assertion checks that the timestamps are in strictly increasing order after the third batch of updates. This ensures that the
last_modified_at
field is correctly updated.
1732-1733
: New test function forlast_modified_at
updates.The
test_set_values_last_modified_at
function is a new addition. Ensure it comprehensively tests thelast_modified_at
field updates.
1746-1747
: Capture initiallast_modified_at
state.The
lma1
variable captures the initial state of thelast_modified_at
field. Ensure this is done before any modifications to maintain accuracy.
1750-1751
: Capturelast_modified_at
after setting values.The
lma2
variable captures the state after setting values. Ensure the updates are correctly reflected in the timestamps.
1752-1755
: Assert correct timestamp updates.The assertion checks that the timestamps are correctly updated. This ensures that the
last_modified_at
field is functioning as expected.
1759-1760
: Capture initiallast_modified_at
state for view.The
lma1
variable captures the initial state of thelast_modified_at
field for the view. Ensure this is done before any modifications to maintain accuracy.
1764-1765
: Capturelast_modified_at
after setting values in view.The
lma2
variable captures the state after setting values in the view. Ensure the updates are correctly reflected in the timestamps.
1766-1769
: Assert correct timestamp updates for view.The assertion checks that the timestamps are correctly updated for the view. This ensures that the
last_modified_at
field is functioning as expected.
1771-1772
: New test function forlast_modified_at
updates in video frames.The
test_set_values_video_last_modified_at
function is a new addition. Ensure it comprehensively tests thelast_modified_at
field updates in video frames.
1789-1790
: Capture initiallast_modified_at
state for frames.The
lma1
variable captures the initial state of thelast_modified_at
field for frames. Ensure this is done before any modifications to maintain accuracy.
1793-1794
: Capturelast_modified_at
after setting values in frames.The
lma2
variable captures the state after setting values in frames. Ensure the updates are correctly reflected in the timestamps.
1795-1798
: Assert correct timestamp updates for frames.The assertion checks that the timestamps are correctly updated for frames. This ensures that the
last_modified_at
field is functioning as expected.
1802-1803
: Capture initiallast_modified_at
state for frames view.The
lma1
variable captures the initial state of thelast_modified_at
field for the frames view. Ensure this is done before any modifications to maintain accuracy.
1807-1808
: Capturelast_modified_at
after setting values in frames view.The
lma2
variable captures the state after setting values in the frames view. Ensure the updates are correctly reflected in the timestamps.
1809-1812
: Assert correct timestamp updates for frames view.The assertion checks that the timestamps are correctly updated for the frames view. This ensures that the
last_modified_at
field is functioning as expected.docs/source/user_guide/using_datasets.rst (28)
973-1001
: Document the new default fields clearly.The new
created_at
andlast_modified_at
fields are well-documented, but ensure that users understand their read-only nature and automatic population.
1002-1006
: Highlight the read-only nature ofcreated_at
andlast_modified_at
fields.The note clarifies that these fields are read-only and automatically populated/updated, which is crucial information for users.
1025-1026
: Ensure new fields are included in examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their usage.Also applies to: 1040-1040
1074-1076
: Include new fields in schema examples.The schema examples correctly include the new
created_at
andlast_modified_at
fields, ensuring users understand their presence in the schema.Also applies to: 1095-1100
1136-1142
: Ensure new fields are included in examples.The examples correctly include the new
created_at
andlast_modified_at
fields in the schema, demonstrating their presence when adding new fields.
1437-1447
: Ensure new fields are included in metadata examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence when storing field metadata.
1508-1515
: Document the read-only nature of new fields clearly.The section correctly documents the read-only nature of the
created_at
andlast_modified_at
fields, providing clear examples.
1516-1531
: Provide clear examples for read-only fields.The examples clearly demonstrate the read-only nature of the
created_at
andlast_modified_at
fields, showing what happens when users try to modify them.
1533-1564
: Document how to mark additional fields as read-only.The section provides clear instructions and examples on how to mark additional fields as read-only, enhancing user control over field accessibility.
1566-1571
: Ensure users save changes after marking fields as read-only.The note correctly emphasizes the need to save changes after marking fields as read-only, ensuring that the changes are persisted.
1573-1584
: Clarify that read-only fields do not interfere with sample operations.The section clarifies that read-only fields do not interfere with adding or deleting samples, providing important context for users.
1585-1605
: Provide examples for reverting read-only fields to editable.The examples clearly demonstrate how to revert read-only fields to editable, providing flexibility for users.
1870-1871
: Include new fields in datetime examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their usage with datetime values.Also applies to: 1875-1876
Line range hint
1900-1909
:
Ensure new fields are included in datetime examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their usage with datetime values.
2000-2001
: Include new fields in regression examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in regression samples.
2056-2057
: Include new fields in classification examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in classification samples.
2127-2128
: Include new fields in multilabel classification examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in multilabel classification samples.
2246-2247
: Include new fields in object detection examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in object detection samples.
2367-2368
: Include new fields in instance segmentation examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in instance segmentation samples.
2503-2504
: Include new fields in polyline examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in polyline samples.
2910-2911
: Include new fields in semantic segmentation examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in semantic segmentation samples.
3012-3013
: Include new fields in heatmap examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in heatmap samples.
3172-3173
: Include new fields in temporal detection examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in temporal detection samples.
3425-3426
: Include new fields in geolocation examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in geolocation samples.
4273-4274
: Include new fields in custom embedded document examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in custom embedded documents.
4324-4325
: Include new fields in image dataset examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in image datasets.
4383-4384
: Include new fields in video dataset examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in video datasets.
5024-5025
: Include new fields in point cloud dataset examples.The examples correctly include the new
created_at
andlast_modified_at
fields, demonstrating their presence in point cloud datasets.tests/unittests/dataset_tests.py (4)
481-535
: LGTM!The function
test_last_modified_at
correctly tests the behavior of thelast_modified_at
field during various dataset operations.
2332-2487
: LGTM!The function
test_read_only_fields
thoroughly tests the behavior of read-only fields, ensuring they cannot be modified directly by users.
2489-2665
: LGTM!The function
test_read_only_frame_fields
thoroughly tests the behavior of read-only fields in frames, ensuring they cannot be modified directly by users.
5333-5368
: LGTM!The function
test_set_new_fields
correctly tests the ability to set new fields on samples and ensure they are correctly added to the dataset schema.fiftyone/core/stages.py (9)
101-117
: Placeholder function approved.The
get_edited_fields
function in theViewStage
class is a placeholder and currently returnsNone
. This is acceptable for an abstract base class.
1421-1442
: LGTM!The
get_edited_fields
function in theExcludeLabels
class correctly identifies and returns the fields that may have been edited by the stage.
1828-1837
: LGTM!The
get_edited_fields
function in theFilterField
class correctly handles the frame field check and returns the appropriate field name if applicable.
2359-2368
: LGTM!The
get_edited_fields
function in theFilterLabels
class correctly handles the frame field check and returns the appropriate field name if applicable.
2782-2791
: LGTM!The
get_edited_fields
function in theFilterKeypoints
class correctly handles the frame field check and returns the appropriate field name if applicable.
3971-3980
: LGTM!The
get_edited_fields
function in theLimitLabels
class correctly handles the frame field check and returns the appropriate field name if applicable.
4144-4153
: LGTM!The
get_edited_fields
function in theMapLabels
class correctly handles the frame field check and returns the appropriate field name if applicable.
4320-4329
: LGTM!The
get_edited_fields
function in theSetField
class correctly handles the frame field check and returns the appropriate field name if applicable.
6545-6566
: LGTM!The
get_edited_fields
function in theSelectLabels
class correctly identifies and returns the fields that may have been edited by the stage.fiftyone/core/dataset.py (7)
690-692
: LGTM!The
last_modified_at
property is correctly added to theDataset
class.
Line range hint
1220-1257
:
LGTM!The
read_only
parameter is correctly integrated into theget_field_schema
method.
Line range hint
1267-1310
:
LGTM!The
read_only
parameter is correctly integrated into theget_frame_field_schema
method.
Line range hint
1325-1373
:
LGTM!The
read_only
parameter is correctly integrated into theadd_sample_field
method.
Line range hint
1471-1516
:
LGTM!The
read_only
parameter is correctly integrated into theadd_frame_field
method.
Line range hint
1614-1638
:
LGTM!The
read_only
parameter is correctly integrated into theadd_group_field
method.
Line range hint
2787-2808
:
LGTM!The
created_at
andlast_modified_at
parameters are correctly integrated into the_make_dict
method.fiftyone/core/collections.py (30)
11-11
: Import statement is correct.The import of
datetime
is necessary for timestamp functionality.
571-585
: Methodsync_last_modified_at
looks good.The method is well-documented and handles the synchronization of
last_modified_at
property effectively.
586-589
: Method_sync_samples_last_modified_at
looks good.The method correctly handles the synchronization of sample timestamps.
1233-1235
: Method_is_read_only_field
looks good.The method correctly determines if a field is read-only.
1237-1254
: Methodget_field
looks good.The addition of the
read_only
parameter is consistent with the new functionality.
1287-1302
: Field validation logic looks good.The calls to
validate_constraints
andvalidate_field
ensure that the field matches the provided constraints.
1311-1311
: Method_parse_field
looks good.The method correctly handles the parsing of fields, including read-only status.
1333-1335
: Method_parse_field
looks good.The method continues to correctly handle the parsing of fields, including read-only status.
1343-1346
: Method_parse_field
looks good.The method continues to correctly handle the parsing of fields, including read-only status.
1362-1362
: Method_parse_field
looks good.The method continues to correctly handle the parsing of fields, including read-only status.
Line range hint
1368-1386
:
Methodget_field_schema
looks good.The addition of the
read_only
parameter is consistent with the new functionality.
1399-1399
: Methodget_field_schema
looks good.The method continues to correctly handle the parsing of fields, including read-only status.
1415-1416
: Methodget_field_schema
looks good.The method continues to correctly handle the parsing of fields, including read-only status.
1797-1801
: Method_edit_sample_tags
looks good.The method correctly checks for read-only fields and updates the
last_modified_at
timestamp.
1840-1846
: Method_edit_label_tags
looks good.The method correctly checks for read-only fields and updates the
last_modified_at
timestamp.
1900-1906
: Method_edit_label_tags
looks good.The method continues to correctly check for read-only fields and updates the
last_modified_at
timestamp.
1926-1931
: Method_edit_label_tags
looks good.The method continues to correctly check for read-only fields and updates the
last_modified_at
timestamp.
1941-1941
: Method_edit_label_tags
looks good.The method continues to correctly check for read-only fields and updates the
last_modified_at
timestamp.
1955-1967
: Method_edit_label_tags
looks good.The method continues to correctly check for read-only fields and updates the
last_modified_at
timestamp.
2396-2398
: Method_set_values
looks good.The method correctly checks for read-only fields.
2555-2557
: Method_set_values
looks good.The method continues to correctly check for read-only fields.
2907-2911
: Method_set_doc_values
looks good.The method correctly checks for read-only fields and updates the
last_modified_at
timestamp.
2925-2930
: Method_set_doc_values
looks good.The method continues to correctly check for read-only fields and updates the
last_modified_at
timestamp.
2950-2954
: Method_set_list_values_by_id
looks good.The method correctly checks for read-only fields and updates the
last_modified_at
timestamp.
2991-2991
: Method_set_list_values_by_id
looks good.The method continues to correctly check for read-only fields and updates the
last_modified_at
timestamp.
3012-3016
: Method_set_label_list_values
looks good.The method correctly checks for read-only fields and updates the
last_modified_at
timestamp.
3037-3037
: Method_set_label_list_values
looks good.The method continues to correctly check for read-only fields and updates the
last_modified_at
timestamp.
3048-3058
: Method_set_labels
looks good.The method correctly checks for read-only fields and updates the
last_modified_at
timestamp.
3083-3083
: Method_set_labels
looks good.The method continues to correctly
591-634
: Method_sync_dataset_last_modified_at
looks good, but avoid bareexcept
.The method correctly handles the synchronization of dataset timestamps. However, using a bare
except
is not recommended. Specify the exception type to catch.- except: + except StopIteration:Likely invalid or redundant comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- fiftyone/core/frame.py (10 hunks)
- fiftyone/migrations/revisions/v0_26_0.py (1 hunks)
- package/desktop/setup.py (1 hunks)
- setup.py (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- fiftyone/core/frame.py
- package/desktop/setup.py
- setup.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- fiftyone/migrations/revisions/v0_26_0.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- fiftyone/migrations/revisions/v0_26_0.py
|
||
def _add_created_at(db, dataset_name, collection_name): | ||
try: | ||
pipeline = [{"$set": {"created_at": {"$toDate": "$_id"}}}] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pulling the timestamp from the id. Very cool 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (3)
tests/unittests/dataset_tests.py (1)
481-535
: Consider adding comments for clarity.Adding comments to explain the purpose of timestamp comparisons would improve readability and maintainability.
+ # Ensure timestamps are correctly updated after modifying dataset info
fiftyone/core/dataset.py (2)
Line range hint
7294-7299
: Avoid bareexcept
clauses.Use a specific exception type instead of a bare
except
to improve error handling clarity.- except: + except Exception:Tools
Ruff
7281-7281: Do not use bare
except
(E722)
7295-7295: Do not use bare
except
(E722)
Line range hint
7294-7299
: Avoid bareexcept
clauses.Use a specific exception type instead of a bare
except
to improve error handling clarity.- except: + except Exception:Tools
Ruff
7281-7281: Do not use bare
except
(E722)
7295-7295: Do not use bare
except
(E722)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (12)
- docs/source/user_guide/using_datasets.rst (41 hunks)
- fiftyone/core/collections.py (34 hunks)
- fiftyone/core/dataset.py (78 hunks)
- fiftyone/core/fields.py (46 hunks)
- fiftyone/core/frame.py (9 hunks)
- fiftyone/core/odm/dataset.py (4 hunks)
- fiftyone/core/odm/mixins.py (29 hunks)
- fiftyone/core/odm/utils.py (8 hunks)
- fiftyone/core/utils.py (2 hunks)
- fiftyone/core/view.py (18 hunks)
- fiftyone/utils/data/importers.py (7 hunks)
- tests/unittests/dataset_tests.py (48 hunks)
Additional context used
Ruff
fiftyone/core/frame.py
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
fiftyone/core/fields.py
188-189: Use a single
if
statement instead of nestedif
statementsCombine
if
statements usingand
(SIM102)
282-285: Use ternary operator
before_kwargs = kwargs if mode in ("before", "both") else {}
instead ofif
-else
-blockReplace
if
-else
-block withbefore_kwargs = kwargs if mode in ("before", "both") else {}
(SIM108)
287-290: Use ternary operator
after_kwargs = kwargs if mode in ("after", "both") else {}
instead ofif
-else
-blockReplace
if
-else
-block withafter_kwargs = kwargs if mode in ("after", "both") else {}
(SIM108)
fiftyone/core/odm/mixins.py
121-123: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
296-296: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
tests/unittests/dataset_tests.py
1515-1515: Do not assign a
lambda
expression, use adef
Rewrite
key_fcn
as adef
(E731)
1584-1584: Found useless expression. Either assign it to a variable or remove it.
(B018)
1586-1586: Found useless expression. Either assign it to a variable or remove it.
(B018)
2758-2758: Found useless expression. Either assign it to a variable or remove it.
(B018)
2769-2769: Found useless expression. Either assign it to a variable or remove it.
(B018)
3049-3049: Found useless expression. Either assign it to a variable or remove it.
(B018)
3060-3060: Found useless expression. Either assign it to a variable or remove it.
(B018)
fiftyone/core/dataset.py
1254-1254: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
3454-3454: Ambiguous variable name:
l
(E741)
3456-3456: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
3653-3656: Use ternary operator
doc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls
instead ofif
-else
-blockReplace
if
-else
-block withdoc_cls = self._frame_doc_cls if is_frame_field else self._sample_doc_cls
(SIM108)
7281-7281: Do not use bare
except
(E722)
fiftyone/core/collections.py
652-652: Do not use bare
except
(E722)
Additional comments not posted (163)
fiftyone/core/odm/utils.py (3)
Line range hint
187-235
:
Integration ofread_only
parameter looks good.The
create_field
function correctly includes theread_only
parameter in its signature and documentation. The parameter is properly added to thefield_kwargs
dictionary, enhancing the field configurability.
Line range hint
304-321
:
Handling ofread_only
attribute is correct.The
get_field_kwargs
function correctly includes theread_only
attribute in the returned dictionary, ensuring that this property is preserved during field processing.
340-354
: New function_parse_field_str
is well-implemented.The
_parse_field_str
function correctly parses string representations of fields, extracting the field type and associated parameters. This enhances the flexibility of field handling.fiftyone/core/odm/dataset.py (2)
Line range hint
70-130
:
Integration ofread_only
field inSampleFieldDocument
is correct.The
read_only
field is properly added to the class and integrated into theto_field
andfrom_field
methods, enhancing the management of sample fields.
622-622
: Addition oflast_modified_at
field inDatasetDocument
is correct.The
last_modified_at
field is properly added to the class, enhancing the dataset's metadata capabilities by tracking modification timestamps.fiftyone/core/frame.py (4)
306-311
: Integration of_enforce_read_only
parameter inadd_frame
is correct.The
_enforce_read_only
parameter is properly added to the method, allowing for more granular control over field modifications.
Line range hint
648-670
:
Integration of timestamp parameters in_make_dict
is correct.The
created_at
andlast_modified_at
parameters are properly added to the method, enhancing its ability to track frame metadata.
Line range hint
722-792
:
Refactoring of_save_replacements
is efficient and flexible.The method is correctly refactored to handle filtered fields and leverage the superclass's functionality, enhancing efficiency and flexibility.
Tools
Ruff
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
798-824
: Implementation of_validate_frame
enhances validation clarity.The method simplifies the validation process by focusing on individual frames, improving clarity and maintainability.
Tools
Ruff
814-818: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
fiftyone/core/fields.py (12)
Line range hint
26-89
: LGTM! Thevalidate_constraints
function handles theread_only
parameter correctly.The addition of the
read_only
parameter is well-integrated into the existing logic.
Line range hint
92-128
: LGTM! Thematches_constraints
function correctly checks theread_only
status.The integration of the
read_only
parameter is consistent with the function's purpose.
Line range hint
132-194
: LGTM! Thevalidate_field
function correctly enforces theread_only
constraint.The addition of the
read_only
parameter is well-handled.
203-218
: LGTM! Theget_field_metadata
function correctly includes theread_only
attribute.The function retrieves metadata as expected.
224-309
: LGTM! Thefilter_schema
function correctly applies theread_only
filter.The integration of the
read_only
parameter enhances the function's flexibility.Tools
Ruff
282-285: Use ternary operator
before_kwargs = kwargs if mode in ("before", "both") else {}
instead ofif
-else
-blockReplace
if
-else
-block withbefore_kwargs = kwargs if mode in ("before", "both") else {}
(SIM108)
287-290: Use ternary operator
after_kwargs = kwargs if mode in ("after", "both") else {}
instead ofif
-else
-blockReplace
if
-else
-block withafter_kwargs = kwargs if mode in ("after", "both") else {}
(SIM108)
311-357
: LGTM! Theflatten_schema
function correctly handles theread_only
parameter.The function performs as expected with the new parameter.
Line range hint
361-398
: LGTM! The_flatten
function correctly incorporates theread_only
parameter.The function uses the
matches_constraints
function effectively.Tools
Ruff
282-285: Use ternary operator
before_kwargs = kwargs if mode in ("before", "both") else {}
instead ofif
-else
-blockReplace
if
-else
-block withbefore_kwargs = kwargs if mode in ("before", "both") else {}
(SIM108)
287-290: Use ternary operator
after_kwargs = kwargs if mode in ("after", "both") else {}
instead ofif
-else
-blockReplace
if
-else
-block withafter_kwargs = kwargs if mode in ("after", "both") else {}
(SIM108)
Line range hint
409-530
: LGTM! TheField
class correctly integrates theread_only
attribute.The property provides clear access to the
read_only
status.
587-594
: LGTM! TheIntField
class correctly includes theread_only
attribute.The integration is consistent with other field classes.
609-616
: LGTM! TheObjectIdField
class correctly includes theread_only
attribute.The integration is consistent with other field classes.
637-644
: LGTM! TheUUIDField
class correctly includes theread_only
attribute.The integration is consistent with other field classes.
653-660
: LGTM! TheBooleanField
class correctly includes theread_only
attribute.The integration is consistent with other field classes.
fiftyone/core/odm/mixins.py (10)
Line range hint
132-159
: LGTM! Theset_field
function correctly enforces the read-only constraint.The
_enforce_read_only
parameter is well-integrated.Tools
Ruff
121-123: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
Line range hint
177-226
: LGTM! Theget_field_schema
function correctly integrates theread_only
parameter.The function performs as expected with the new parameter.
Line range hint
234-324
: LGTM! Themerge_field_schema
function correctly handles theoverwrite
parameter.The integration enhances the flexibility of schema merging.
Tools
Ruff
296-296: Use
key in dict
instead ofkey in dict.keys()
Remove
.keys()
(SIM118)
Line range hint
338-393
: LGTM! Theadd_field
function correctly integrates theread_only
parameter.The function allows fields to be marked as read-only upon creation.
Line range hint
454-466
: LGTM! The_create_field
function correctly integrates theread_only
parameter.The function allows fields to be created with the
read_only
attribute.
Line range hint
1059-1191
: LGTM! The_merge_field
function correctly handles theoverwrite
parameter.The integration enhances the flexibility of field merging.
1433-1439
: LGTM! The_insert
function correctly sets timestamps forcreated_at
andlast_modified_at
.The integration ensures accurate tracking of document creation and modification.
1447-1457
: LGTM! The_update
function correctly updates thelast_modified_at
timestamp.The integration ensures accurate tracking of document modifications.
Line range hint
671-851
: LGTM! TheDatasetMixin
class correctly handles read-only fields and timestamp updates.The integration enhances the functionality of the class.
117-123
: Useraise ... from None
in exception handling.To distinguish this from errors in exception handling, use
raise ... from None
.- raise AttributeError( - "%s has no field '%s'" % (self._doc_name(), field_name) - ) + raise AttributeError( + "%s has no field '%s'" % (self._doc_name(), field_name) + ) from NoneLikely invalid or redundant comment.
Tools
Ruff
121-123: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling(B904)
fiftyone/core/view.py (10)
21-23
: Import addition approved.The addition of
ViewField
fromfiftyone.core.expressions
aligns with its purpose and is consistent with handling expressions in views.
200-202
: Property addition approved.The
_frame_cls
property enhances the class's capabilities to manage frame-related data by returningfofr.FrameView
.
537-537
: Method invocation approved.The invocation of
_make_sample
within_iter_samples
aligns with the intended functionality and improves clarity.
548-552
: Method definition approved.The
_make_sample
method replaces_make_sample_fcn
, aligning with the intended functionality and improving clarity in naming.
554-558
: Method definition approved.The
_make_frame
method mirrors_make_sample
, aligning with the intended functionality and providing a parallel structure for handling frames.
586-610
: Method definition approved.The
_init_make_frame
method encapsulates the logic for creating frame instances and includes robust error handling.
846-846
: Query enhancement approved.The use of
F(id_field)
enhances querying capabilities in theget_group
method.
Line range hint
917-962
: Schema retrieval enhancement approved.The addition of
read_only
andmode
parameters inget_field_schema
enhances flexibility in schema retrieval.
Line range hint
968-1018
: Frame schema retrieval enhancement approved.The addition of
read_only
andmode
parameters inget_frame_field_schema
enhances flexibility in frame schema retrieval.
1850-1865
: Method addition approved.The
_get_edited_fields
method provides an effective mechanism to track changes across view stages.fiftyone/core/utils.py (3)
2156-2156
: Initialization of_new_kwargs
is appropriate.The initialization of
_new_kwargs
in the__init__
method is correct and prepares the context manager for tracking new attributes.
2160-2165
: Logic for tracking new attributes is sound.The use of
_new_kwargs
to track new attributes separately from existing ones is correctly implemented.
2175-2176
: Cleanup of new attributes is appropriate.The removal of new attributes using
_new_kwargs
in the__exit__
method ensures that temporary changes do not persist.fiftyone/utils/data/importers.py (5)
8-8
: Import statement fordatetime
is appropriate.The addition of
datetime
is necessary for handling timestamps in the code.
403-403
: Method name change improves clarity.Renaming
_apply_field_schema
to_apply_sample_field_schema
clarifies the method's purpose.
1052-1053
: Return type description update is clear.The updated description provides clearer information about the return type of
get_sample_field_schema
.
1952-1953
: Addition of timestamp fields for frames is appropriate.The
created_at
andlast_modified_at
fields ensure accurate timestamping for imported frames.Verify that these fields are correctly populated and used throughout the codebase.
Line range hint
1907-1924
: Addition of timestamp fields is appropriate.The
created_at
andlast_modified_at
fields ensure accurate timestamping for imported samples.Verify that these fields are correctly populated and used throughout the codebase.
Verification successful
Verification of Timestamp Fields Usage
The
created_at
andlast_modified_at
fields are extensively used across the codebase, particularly in unit tests and various modules. This indicates that the fields are well-integrated and consistently utilized for timestamping purposes.
- Files with Usage:
tests/unittests/patches_tests.py
tests/unittests/view_tests.py
tests/unittests/index_tests.py
tests/unittests/group_tests.py
tests/unittests/import_export_tests.py
tests/unittests/video_tests.py
tests/unittests/dataset_tests.py
fiftyone/utils/data/importers.py
fiftyone/core/dataset.py
fiftyone/core/odm/sample.py
fiftyone/core/odm/frame.py
The presence of these fields in tests and various modules confirms their correct implementation and usage.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `created_at` and `last_modified_at` fields are used correctly. # Test: Search for the usage of `created_at` and `last_modified_at`. Expect: Consistent usage across the codebase. rg --type python -A 5 $'created_at|last_modified_at'Length of output: 84
Script:
#!/bin/bash # Description: Verify that `created_at` and `last_modified_at` fields are used correctly. # Test: Search for the usage of `created_at` and `last_modified_at`. Expect: Consistent usage across the codebase. rg --type py -A 5 $'created_at|last_modified_at'Length of output: 193878
docs/source/user_guide/using_datasets.rst (6)
992-1006
: Addition ofcreated_at
andlast_modified_at
fields is appropriate.The
created_at
andlast_modified_at
fields are correctly added as read-only fields, automatically populated, which aligns with the PR objectives.
1025-1026
: Example correctly demonstrates new fields.The inclusion of
created_at
andlast_modified_at
fields in the example aligns with the updated default sample fields.
Line range hint
1074-1100
: Field schema updates are consistent.The field schema output correctly includes
created_at
andlast_modified_at
fields, reflecting the documentation changes.
Line range hint
1223-1251
: Schema output correctly includes new fields.The dataset field schema output appropriately includes
created_at
andlast_modified_at
, aligning with the documentation updates.
1439-1506
: Metadata storage documentation is a valuable addition.The section on storing field metadata, including descriptions, enhances the documentation by providing users with useful information on managing field metadata.
1508-1604
: Read-only fields section is well-explained.The documentation on read-only fields, including
created_at
andlast_modified_at
, effectively communicates their constraints and management, which is crucial for users.tests/unittests/dataset_tests.py (2)
5120-5174
: LGTM! The synchronization tests are comprehensive.The
test_sync_last_modified_at
function effectively verifies the synchronization of thelast_modified_at
field.
5335-5370
: LGTM! The test for new fields is well-implemented.The
test_set_new_fields
function accurately tests the addition of new fields and the behavior of dynamic nested fields.fiftyone/core/dataset.py (73)
34-34
: LGTM!The import of
ViewField
asF
is consistent with its usage in the file.
Line range hint
60-65
: LGTM!The
DatasetNotFoundError
class is well-defined and inherits fromValueError
appropriately.
374-376
: LGTM!The
__delitem__
function correctly delegates todelete_samples
.
689-692
: LGTM!The
last_modified_at
property is correctly implemented to return the dataset's last modified timestamp.
1281-1284
: LGTM!The
get_field_schema
method correctly includes theread_only
parameter for handling read-only fields.
1325-1328
: LGTM!The
get_frame_field_schema
method correctly includes theread_only
parameter for handling read-only fields.
1379-1382
: LGTM!The
add_sample_field
method correctly includes theread_only
parameter for marking fields as read-only.
1525-1528
: LGTM!The
add_frame_field
method correctly includes theread_only
parameter for marking fields as read-only.
1673-1676
: LGTM!The
add_group_field
method correctly includes theread_only
parameter for marking fields as read-only.
2099-2104
: LGTM!The
_remove_dynamic_sample_fields
method is correctly implemented to purge and reload sample fields.
2130-2135
: LGTM!The
_remove_dynamic_frame_fields
method is correctly implemented to purge and reload frame fields.
Line range hint
2376-2380
: LGTM!The
_iter_samples
method is correctly implemented to iterate over samples and yield them.
Line range hint
2503-2508
: LGTM!The
_iter_groups
method is correctly implemented to iterate over groups and yield samples.
2832-2838
: LGTM!The
_make_dict
method correctly handles timestamps and converts samples to MongoDB dictionaries.
2832-2838
: LGTM!The
_bulk_write
method is correctly implemented for handling bulk writing operations.
3348-3351
: LGTM!The
_is_read_only_field
method is correctly implemented to check if a field is read-only.
9543-9547
: LGTM!The
_set_field_read_only
method is correctly implemented to set the read-only status of fields recursively.
8335-8363
: LGTM!The
_clone_extras
method is correctly implemented to clone various dataset components.
7859-7878
: LGTM!The
_clone_collection
method is correctly implemented for cloning dataset collections with timestamp handling.
7938-7947
: LGTM!The
_get_samples_pipeline
method is correctly implemented to construct a pipeline for sample operations.
7953-7962
: LGTM!The
_get_frames_pipeline
method is correctly implemented to construct a pipeline for frame operations.
8046-8059
: LGTM!The
_get_edited_fields
method is correctly implemented to retrieve edited fields from the dataset.
7167-7170
: LGTM!The
_apply_sample_field_schema
method is correctly implemented to apply a schema to sample fields.
7172-7175
: LGTM!The
_apply_frame_field_schema
method is correctly implemented to apply a schema to frame fields.
7274-7277
: LGTM!The
_make_sample
method is correctly implemented to convert a dictionary to a sample object.
7288-7290
: LGTM!The
_make_frame
method is correctly implemented to convert a dictionary to a frame object.
9493-9496
: LGTM!The
_discard_none
method is correctly implemented to removeNone
values from a list.
9497-9500
: LGTM!The
_parse_fields
method is correctly implemented to parse field names into a list.
9548-9551
: LGTM!The
_extract_archive_if_necessary
method is correctly implemented to handle archive extraction.
8335-8345
: LGTM!The
_clone_reference_doc
method is correctly implemented to clone reference documents.
8363-8366
: LGTM!The
_clone_run
method is correctly implemented to clone run documents.
9492-9493
: LGTM!The
_get_random_characters
method is correctly implemented to generate random characters.
7513-7514
: LGTM!The
_validate_dataset_name
method is correctly implemented to validate dataset names.
7513-7514
: LGTM!The
_make_sample_collection_name
method is correctly implemented to generate a sample collection name.
4135-4136
: LGTM!The
_get_workspace_doc
method is correctly implemented to retrieve workspace documents.
3890-3891
: LGTM!The
_get_saved_view_doc
method is correctly implemented to retrieve saved view documents.
3890-3891
: LGTM!The
_load_saved_view_from_doc
method is correctly implemented to load saved views from documents.
7440-7441
: LGTM!The
_update_last_loaded_at
method is correctly implemented to update the last loaded timestamp.
2135-2136
: LGTM!The
_reload_docs
method is correctly implemented to reload documents.
2104-2105
: LGTM!The
_purge_fields
method is correctly implemented to purge fields from a collection.
1361-1362
: LGTM!The
_has_frame_fields
method is correctly implemented to check for frame fields in a dataset.
1312-1318
: LGTM!The
_get_field_schema
method is correctly implemented to retrieve the field schema for a dataset.
1325-1331
: LGTM!The
_get_frame_field_schema
method is correctly implemented to retrieve the frame field schema for a dataset.
3359-3360
: LGTM!The
_get_label_field_path
method is correctly implemented to retrieve the path for a label field.
3361-3362
: LGTM!The
_handle_frame_field
method is correctly implemented to process frame fields.
3361-3362
: LGTM!The
_get_label_field_root
method is correctly implemented to retrieve the root for a label field.
7938-7947
: LGTM!The
_get_samples_pipeline
method is correctly implemented to construct a pipeline for sample operations.
7953-7962
: LGTM!The
_get_frames_pipeline
method is correctly implemented to construct a pipeline for frame operations.
8046-8059
: LGTM!The
_get_edited_fields
method is correctly implemented to retrieve edited fields from the dataset.
7167-7170
: LGTM!The
_apply_sample_field_schema
method is correctly implemented to apply a schema to sample fields.
7172-7175
: LGTM!The
_apply_frame_field_schema
method is correctly implemented to apply a schema to frame fields.
7274-7277
: LGTM!The
_make_sample
method is correctly implemented to convert a dictionary to a sample object.
7288-7290
: LGTM!The
_make_frame
method is correctly implemented to convert a dictionary to a frame object.
9493-9496
: LGTM!The
_discard_none
method is correctly implemented to removeNone
values from a list.
9497-9500
: LGTM!The
_parse_fields
method is correctly implemented to parse field names into a list.
9548-9551
: LGTM!The
_extract_archive_if_necessary
method is correctly implemented to handle archive extraction.
8335-8345
: LGTM!The
_clone_reference_doc
method is correctly implemented to clone reference documents.
8363-8366
: LGTM!The
_clone_run
method is correctly implemented to clone run documents.
9492-9493
: LGTM!The
_get_random_characters
method is correctly implemented to generate random characters.
7513-7514
: LGTM!The
_validate_dataset_name
method is correctly implemented to validate dataset names.
7513-7514
: LGTM!The
_make_sample_collection_name
method is correctly implemented to generate a sample collection name.
4135-4136
: LGTM!The
_get_workspace_doc
method is correctly implemented to retrieve workspace documents.
3890-3891
: LGTM!The
_get_saved_view_doc
method is correctly implemented to retrieve saved view documents.
3890-3891
: LGTM!The
_load_saved_view_from_doc
method is correctly implemented to load saved views from documents.
7440-7441
: LGTM!The
_update_last_loaded_at
method is correctly implemented to update the last loaded timestamp.
2135-2136
: LGTM!The
_reload_docs
method is correctly implemented to reload documents.
2104-2105
: LGTM!The
_purge_fields
method is correctly implemented to purge fields from a collection.
1361-1362
: LGTM!The
_has_frame_fields
method is correctly implemented to check for frame fields in a dataset.
1312-1318
: LGTM!The
_get_field_schema
method is correctly implemented to retrieve the field schema for a dataset.
1325-1331
: LGTM!The
_get_frame_field_schema
method is correctly implemented to retrieve the frame field schema for a dataset.
3359-3360
: LGTM!The
_get_label_field_path
method is correctly implemented to retrieve the path for a label field.
3361-3362
: LGTM!The
_handle_frame_field
method is correctly implemented to process frame fields.
3361-3362
: LGTM!The
_get_label_field_root
method is correctly implemented to retrieve the root for a label field.fiftyone/core/collections.py (33)
11-11
: Import addition is appropriate.The addition of
datetime
is consistent with the need for timestamp handling in the file.
1233-1235
: Method addition is appropriate.The
_is_read_only_field
method correctly determines if a field is read-only using_parse_field
.
1275-1276
: Docstring update is accurate.The addition of the
read_only
parameter in the docstring is clear and informative.
1287-1290
: Constraints validation is well-integrated.The
read_only
parameter is correctly included in the constraints validation, enhancing the method's robustness.
1311-1311
: Return statement update is consistent.The inclusion of
read_only
in the return values aligns with the method's purpose.
1333-1334
: Initialization is necessary.The initialization of
read_only
toNone
is necessary for its subsequent assignment.
1346-1346
: Assignment is logical.The assignment of
read_only
based on the field attribute ensures correct status capture.
1362-1362
: Return statement update is consistent.Including
read_only
in the return values aligns with the method's purpose.
1386-1393
: Docstring update is accurate.The addition of
read_only
andmode
parameters in the docstring is clear and informative.
1422-1431
: Docstring update is accurate.The addition of
read_only
andmode
parameters in the docstring is clear and informative.
1455-1456
: Return statement update is consistent.The update aligns with the method's purpose of returning field paths and instances.
1476-1478
: Return statement update is consistent.The update aligns with the method's purpose of returning field paths and instances.
1809-1810
: Read-only check is necessary.The check prevents unauthorized modifications to the
tags
field.
1854-1857
: Read-only checks are necessary.These checks ensure that modifications are not made to protected fields.
1914-1917
: Read-only checks are necessary.These checks ensure that modifications are not made to protected fields.
1939-1940
: Read-only check is necessary.The check prevents unauthorized modifications to label fields.
1953-1953
: Timestamp update is consistent.Updating
last_modified_at
during label modifications ensures accurate tracking.
1979-1979
: Timestamp update is consistent.Updating
last_modified_at
during label modifications ensures accurate tracking.
2408-2409
: Read-only check is necessary.The check prevents unauthorized modifications to fields.
2567-2568
: Read-only check is necessary.The check prevents unauthorized modifications to fields.
2919-2923
: Read-only check and timestamp update are necessary.The check prevents unauthorized modifications, and updating
last_modified_at
ensures accurate tracking.
2937-2941
: Timestamp update is consistent.Updating
last_modified_at
during modifications ensures accurate tracking.
2962-2966
: Read-only check and timestamp update are necessary.The check prevents unauthorized modifications, and updating
last_modified_at
ensures accurate tracking.
3003-3003
: Timestamp update is consistent.Updating
last_modified_at
during modifications ensures accurate tracking.
3024-3028
: Read-only check and timestamp update are necessary.The check prevents unauthorized modifications, and updating
last_modified_at
ensures accurate tracking.
3049-3049
: Timestamp update is consistent.Updating
last_modified_at
during modifications ensures accurate tracking.
3060-3070
: Read-only check and timestamp update are necessary.The check prevents unauthorized modifications, and updating
last_modified_at
ensures accurate tracking.
3095-3095
: Timestamp update is consistent.Updating
last_modified_at
during modifications ensures accurate tracking.
3109-3109
: Timestamp update is consistent.Updating
last_modified_at
during modifications ensures accurate tracking.
9442-9458
: Index update is consistent.Including
created_at
andlast_modified_at
in the default indexes optimizes queries involving timestamps.
9468-9499
: Index update is consistent.Including
created_at
andlast_modified_at
in the default indexes optimizes queries involving timestamps.
11057-11058
: Timestamp update is consistent.Updating
last_modified_at
in_parse_frame_values_dicts
ensures accurate tracking of modifications.
11082-11087
: Timestamp update is consistent.Updating
last_modified_at
for new frame numbers ensures accurate tracking of modifications.
@@ -1785,6 +1825,16 @@ def only_matches(self): | |||
"""Whether to only include samples that match the filter.""" | |||
return self._only_matches | |||
|
|||
def get_edited_fields(self, sample_collection, frames=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of these could share use a common mixin/util call, maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah there is code duplication here, but it's also true for some of the other ViewStage
interface methods so I was just following along with the existing broken windows 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nuts and bolts LGTM. I like the new mode
parameter 🚀
9ef797d
to
6233283
Compare
created_at
, last_modified_at
, and read-only fieldscreated_at
, last_modified_at
, and read-only fields
test failures 👀 |
forgot that PRs into other branches don't run the tests 🙃 |
* Fields have created_at attribute
455c9b8
to
cab2429
Compare
created_at
, last_modified_at
, and read-only fieldscreated_at
, last_modified_at
, and read-only fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm - @brimoor are we ready to merge into develop?
@swheaton I've still been pondering in background mode whether we should ship it like this or automatically update One material consequence is that we'd delete Side note: I'm looking at #4765 now. |
hmm ok will think about it. |
Alright, code freeze approaches, this is fine as-is 🚀 |
great thanks |
Change log
created_at
fields toSample
andFrame
last_modified_at
fields toSample
andFrame
Dataset.last_modified_at
propertyField.created_at
propertycreated_at
property to all fieldsImplementation details
As currently implemented,
Dataset.last_modified_at
is not automatically updated in response to changes to its child samples, andSample.last_modified_at
of video samples is not automatically updated in response to modifications to its child frames. Instead, the user must cause this to happen (if they want it) by calling:I chose this path because:
created_at
andlast_modified_at
indexes are created by default (with the assumption that sorting by these fields will be popular), so computing$max
across these collections is efficientlast_modified_at
values upon every downstream update would be a significant performance penalty as it would 2x or 3x (for frame modifications) the number of update operations that must be performed.Migration considerations
The upward migration declares
created_at
andlast_modified_at
fields on all datasets and populates thecreated_at
values via{"$toDate": "$_id"}
, taking advantage of the fact thatObjectId
s encode the original timestamp when they were generated. Since we do not know when the samples were actually last modified, though,last_modified_at
is left asNone
(okay we could also set this to{"$toDate": "$_id"}
as well since we technically edited the sample by settingcreated_at
, but that's not any more informative than leaving it as None). I ensured theseNone
values will not cause problems when edits are later made orsync_last_modified_at()
is later called.The downward migration does not delete the
created_at
andlast_modified_at
fields, since they are still valid fields, they will just no longer be default + read-only fields and thelast_modified_at
field will no longer be automatically updated. However, the assumption is that the user will either want to re-upgrade soon, at which point they'll be glad to have theirlast_modified_at
values retained, or else they can just usedelete_sample_field()
to manually delete these fields.Example usage
created_at
/last_modified_at
Read-only fields